test: introduce a test reproducing the #5583

[janekmi]

This change is 

[grom72]

Reviewed 2 of 2 files at r1, 12 of 12 files at r2, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @janekmi, @osalyk, and @pbalcer)

---

src/test/obj_ulog_advanced/obj_ulog_advanced.c line 20 at r2 (raw file):

/*
 * The persistent shadow log is a DRAM buffer where intially all redo log

The persistent redo log shall be introduced as well.

Code quote:

/*
 * The persistent shadow log is a DRAM buffer where intially all redo log

---

src/test/obj_ulog_advanced/obj_ulog_advanced.c line 23 at r2 (raw file):

 * entries are placed. This log's initial capacity is 1KiB (ULOG_BASE_SIZE)
 * and can be reallocated to grow bigger as necessary. It grows when adding
 * the next entry will reduce the available capacity below CACHELINE_SIZE (64B).

It grows when adding the next entry will reduce the available capacity below CACHELINE_SIZE (64B).

What do you mean?

Code quote:

 * and can be reallocated to grow bigger as necessary. It grows when adding
 * the next entry will reduce the available capacity below CACHELINE_SIZE (64B).

---

src/test/obj_ulog_advanced/obj_ulog_advanced.c line 25 at r2 (raw file):

 * the next entry will reduce the available capacity below CACHELINE_SIZE (64B).
 *
 * When the persistent shadow log is ready, it will be copied to the persistent

When is it ready?

Code quote:

When the persistent shadow log is ready

---

src/test/obj_ulog_advanced/obj_ulog_advanced.c line 39 at r2 (raw file):

 * the persistent shadow log's capacity is the actual capacity of the underlying
 * DRAM buffer (>=1024B) or the capacity of a single persistent redo log
 * (<=640B).

What is the proper answer to not obvious capacity definition?

Code quote:

 * the capacity. It turns out to be confusing since it is not obvious whether
 * the persistent shadow log's capacity is the actual capacity of the underlying
 * DRAM buffer (>=1024B) or the capacity of a single persistent redo log
 * (<=640B).

---

src/test/obj_ulog_advanced/obj_ulog_advanced.c line 180 at r2 (raw file):

 */
static void
publish(const char *path, int slots_num, bool error_inject, bool abort_inject)

Can we use both *_inject in the same function call?

Code quote:

static void
publish(const char *path, int slots_num, bool error_inject, bool abort_inject)

[janekmi]

Reviewed 2 of 2 files at r1, 11 of 12 files at r2, 12 of 12 files at r4, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @grom72, @osalyk, and @pbalcer)

---

src/test/obj_ulog_advanced/obj_ulog_advanced.c line 20 at r2 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

The persistent redo log shall be introduced as well.

Done.

---

src/test/obj_ulog_advanced/obj_ulog_advanced.c line 23 at r2 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

It grows when adding the next entry will reduce the available capacity below CACHELINE_SIZE (64B).

What do you mean?

Done.

---

src/test/obj_ulog_advanced/obj_ulog_advanced.c line 25 at r2 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

When is it ready?

Done.

---

src/test/obj_ulog_advanced/obj_ulog_advanced.c line 39 at r2 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

What is the proper answer to not obvious capacity definition?

Done.

---

src/test/obj_ulog_advanced/obj_ulog_advanced.c line 180 at r2 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

Can we use both *_inject in the same function call?

Yes. The comment proceeding the function exactly says "and/or" so all of the argument combinations are allowed. And all of them are used.

[janekmi]

This test is meant to reproduce the scenario initially suggested as problematic in the DAOS project: https://daosio.atlassian.net/browse/DAOS-13081
I would appreciate comments from all developers involved so far: @NiuYawei, @sherintg, @liw.

The most crucial part is to confirm the scenario is implemented as reported. Because if it is the case and the test is passing successfully it will be a confirmation the problem is non-existent in the PMDK project.

Feel free to invite whoever else you consider interested in the problem.

Thanks!

[pbalcer]

lgtm, just a couple of comments, feel free to ignore.

[pbalcer]

pmreorder should take care of any sort of abort cases, no?

[pbalcer]

I found this confusing as a test :) Essentially the implementation is correct, but this is forcing an incorrect behavior. So what this is testing is whether the test is correct. I know you have a big comment, but I had to spend some time thinking what this is for even though I know what's the problem.

[liw]

Thanks, Jan. 👍

[janekmi]

You are welcome. 👍

[grom72]

Reviewed 7 of 12 files at r3, 3 of 12 files at r4.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @janekmi, @liw, and @osalyk)

---

src/test/obj_ulog_advanced/obj_ulog_advanced.c line 29 at r4 (raw file):

 * and can be reallocated to grow bigger as necessary. If the write offset
 * before adding the new entry + CACHELINE_SIZE (64B) == capacity than
 * the persistent shadow log will grow.

How much each time?

Code quote:

 * the persistent shadow log will grow.

[janekmi]

Reviewed 10 of 10 files at r5, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @grom72, @liw, @osalyk, and @pbalcer)

---

src/test/obj_ulog_advanced/obj_ulog_advanced.c line 29 at r4 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

How much each time?

Done.

---

src/test/obj_ulog_advanced/obj_ulog_advanced.c line 102 at r4 (raw file):

Previously, pbalcer (Piotr Balcer) wrote…

pmreorder should take care of any sort of abort cases, no?

And I believe it does. And since pmreorder_create_store_log does not like when the recorded process terminates abnormally I decided to go with full, normal, uninterrupted recording. Having that I believe pmreorder is able to generate a state of the pool assuming the store sequence has been interrupted at any possible moment.

---

src/test/obj_ulog_advanced/obj_ulog_advanced.c line 115 at r4 (raw file):

Previously, pbalcer (Piotr Balcer) wrote…

I found this confusing as a test :) Essentially the implementation is correct, but this is forcing an incorrect behavior. So what this is testing is whether the test is correct. I know you have a big comment, but I had to spend some time thinking what this is for even though I know what's the problem.

So... I made the comment even bigger. xD I hope it helps. Let me know if you have a better idea. ;-)

[osalyk]

Reviewed 2 of 2 files at r1, 5 of 12 files at r4, 10 of 10 files at r5, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @liw and @pbalcer)

[janekmi]

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @liw and @pbalcer)

[grom72]

Reviewed 1 of 10 files at r5.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @liw and @pbalcer)

---

src/test/obj_ulog_advanced/obj_ulog_advanced.c line 115 at r4 (raw file):

Previously, janekmi (Jan Michalski) wrote…

So... I made the comment even bigger. xD I hope it helps. Let me know if you have a better idea. ;-)

+1

[grom72]

Reviewed 1 of 10 files at r5.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @liw and @pbalcer)

[grom72]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @liw and @pbalcer)

[grom72]

Reviewed 1 of 1 files at r6.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @liw and @pbalcer)

[janekmi]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @liw and @pbalcer)

---

src/test/obj_ulog_advanced/obj_ulog_advanced.c line 115 at r4 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

+1

Done.