test: verify prealloc result to avoid skipping wrong function execution #5568

grom72 · 2023-03-31T16:34:10Z

prealloc might fail in case of pool reopen after transaction abort

This change is

wlemkows · 2023-04-03T06:12:36Z

src/test/RUNTESTS

@@ -354,7 +354,7 @@ testtype=check
 fstype=all
 testconfig="./testconfig.sh"
 killopt="-k 10s"
-runtest_timeout="3m"
+runtest_timeout="30m"


Please do not change the global timeout for all tests. You can change the test type to long in the test case instead.

roll-back done

wlemkows · 2023-04-03T06:14:52Z

src/test/obj_pmalloc_mt/TEST3

@@ -16,6 +16,6 @@ configure_valgrind drd force-enable
 setup

 PMEM_IS_PMEM_FORCE=1 expect_normal_exit\
- ./obj_pmalloc_mt$EXESUFFIX 4 64 4 $DIR/testfile
+ ./obj_pmalloc_mt$EXESUFFIX 4 64 4 $DIR/testfile.TEST3


I suggest using only the number at the end, as we do in other tests.

wlemkows · 2023-04-03T06:17:56Z

src/test/obj_pmalloc_mt/TEST8

+PMEM_IS_PMEM_FORCE=1 expect_normal_exit\
+ ./obj_pmalloc_mt$EXESUFFIX 32 1000 100 $DIR/testfile.TEST8
+
+if test -f "rm $DIR/testfile.TEST8"; then


Please assign path to var at the beginning.
You can always delete this file with -f without the if condition. Is there any reason why you check it?

wlemkows · 2023-04-03T06:18:00Z

src/test/obj_pmalloc_mt/TEST8

+
+require_valgrind 3.10
+require_fs_type pmem
+require_test_type medium


file removed

grom72

Reviewed 2 of 3 files at r1, 3 of 6 files at r7.
Reviewable status: 4 of 9 files reviewed, 4 unresolved discussions (waiting on @wlemkows)

src/test/obj_pmalloc_mt/TEST3 line 19 at r6 (raw file):

Previously, wlemkows (Weronika Lewandowska) wrote…

I suggest using only the number at the end, as we do in other tests.

Done.

src/test/obj_pmalloc_mt/TEST8 line 14 at r6 (raw file):

Previously, wlemkows (Weronika Lewandowska) wrote…

long?

Done.

src/test/obj_pmalloc_mt/TEST8 line 25 at r6 (raw file):

Previously, wlemkows (Weronika Lewandowska) wrote…

Please assign path to var at the beginning.
You can always delete this file with -f without the if condition. Is there any reason why you check it?

Done.

src/test/RUNTESTS line 357 at r6 (raw file):

Previously, wlemkows (Weronika Lewandowska) wrote…

Please do not change the global timeout for all tests. You can change the test type to long in the test case instead.

Done.
It was only experimental as long tests are not run executed per PR.

codecov · 2023-04-03T07:39:18Z

Codecov Report

Merging #5568 (a7dbdd5) into master (bf7df09) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head a7dbdd5 differs from pull request most recent head 0fb9504. Consider uploading reports for the commit 0fb9504 to get more accurate results

@@            Coverage Diff             @@
##           master    #5568      +/-   ##
==========================================
- Coverage   74.27%   74.26%   -0.01%     
==========================================
  Files         145      145              
  Lines       22131    22131              
  Branches     3705     3704       -1     
==========================================
- Hits        16437    16436       -1     
- Misses       5694     5695       +1

lukaszstolarczuk

Reviewed 1 of 2 files at r8.
Reviewable status: 4 of 9 files reviewed, 7 unresolved discussions (waiting on @grom72 and @wlemkows)

-- commits line 30 at r8:
you can squash this change to the first commit (and in general I guess, some cleanup is required in all commits)

src/test/obj_pmalloc_mt/err8.log.match line 1 at r8 (raw file):

{obj_pmalloc_mt.c:66 realloc_worker} obj_pmalloc_mt/TEST8: Error: assertion failure: ret (0xffffffffffffffff) == 0 (0x0)

missing EOF

utils/docker/run-build.sh line 34 at r8 (raw file):

echo "## Running make pycheck"
# do not change -j2 to -j$(nproc) in case of tests (make check/pycheck)
make -j1 pycheck

is this change still required (since you added separate files for each test)?

grom72

Reviewed 3 of 6 files at r7, 2 of 2 files at r9.
Reviewable status: 7 of 9 files reviewed, 7 unresolved discussions (waiting on @lukaszstolarczuk and @wlemkows)

-- commits line 30 at r8:

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

you can squash this change to the first commit (and in general I guess, some cleanup is required in all commits)

Done.

src/test/obj_pmalloc_mt/err8.log.match line 1 at r8 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

missing EOF

Done.

src/test/obj_pmalloc_mt/TEST8 line 14 at r6 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

Done.

short - w/o Valgrind

utils/docker/run-build.sh line 34 at r8 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

is this change still required (since you added separate files for each test)?

Yes, to avoid any interferences between tests.
Otherwise, we get unpredictable test results.

grom72

Reviewed 1 of 1 files at r4.
Reviewable status: 8 of 9 files reviewed, 7 unresolved discussions (waiting on @lukaszstolarczuk and @wlemkows)

grom72

Reviewed 1 of 1 files at r10, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @lukaszstolarczuk and @wlemkows)

lukaszstolarczuk

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @grom72 and @wlemkows)

src/test/obj_pmalloc_mt/TEST8 line 14 at r6 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

short - w/o Valgrind

soo.. remove the require_valgrind commented line above...? 😉 (applies not only to this line, I guess - redundant comments may be removed)

src/test/obj_pmalloc_mt/TEST8 line 25 at r6 (raw file):
the first part not done yet

Please assign path to var at the beginning.

src/test/obj_pmalloc_mt/TEST8 line 7 at r10 (raw file):

#
# src/test/obj_pmalloc_mt/TEST8 -- multithreaded allocator test
#	(long - several iteration) w/ statistics

and also, I believe the suggestion about long was taken from here 😉

utils/docker/run-build.sh line 34 at r8 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

Yes, to avoid any interferences between tests.
Otherwise, we get unpredictable test results.

as discussed it's a good change!
Perhaps just, please, make the changes in this file a separate commit

grom72

Reviewed 1 of 1 files at r11.
Reviewable status: 8 of 9 files reviewed, 6 unresolved discussions (waiting on @lukaszstolarczuk and @wlemkows)

src/test/obj_pmalloc_mt/TEST8 line 14 at r6 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

soo.. remove the require_valgrind commented line above...? 😉 (applies not only to this line, I guess - redundant comments may be removed)

Done.

src/test/obj_pmalloc_mt/TEST8 line 25 at r6 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

the first part not done yet

Please assign path to var at the beginning.

Done.

src/test/obj_pmalloc_mt/TEST8 line 7 at r10 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

and also, I believe the suggestion about long was taken from here 😉

Done.

utils/docker/run-build.sh line 34 at r8 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

as discussed it's a good change!
Perhaps just, please, make the changes in this file a separate commit

Done.

grom72

Reviewed 1 of 1 files at r12.
Reviewable status: all files reviewed (commit messages unreviewed), 6 unresolved discussions (waiting on @lukaszstolarczuk and @wlemkows)

lukaszstolarczuk

Reviewed 4 of 6 files at r7, 1 of 2 files at r9, 1 of 1 files at r10, 1 of 1 files at r12.
Reviewable status: all files reviewed (commit messages unreviewed), 6 unresolved discussions (waiting on @grom72 and @wlemkows)

utils/docker/run-build.sh line 34 at r8 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

Done.

not done...? I can still see a single commit :wink

utils/docker/run-build.sh line 33 at r12 (raw file):

echo ""
echo "## Running make pycheck"
# do not change -j2 to -j$(nproc) in case of tests (make check/pycheck)

you can also update the comment (here and above), e.g.:
# do not change -j1 to more than -j2 for tests (both check/pycheck - they may hang)

lukaszstolarczuk

Reviewable status: all files reviewed (commit messages unreviewed), 6 unresolved discussions (waiting on @grom72 and @wlemkows)

utils/docker/run-build.sh line 33 at r12 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

you can also update the comment (here and above), e.g.:
# do not change -j1 to more than -j2 for tests (both check/pycheck - they may hang)

... thay may hang if more jobs used

grom72

Reviewed 1 of 1 files at r13, 2 of 4 files at r14.
Reviewable status: 7 of 9 files reviewed, 7 unresolved discussions (waiting on @lukaszstolarczuk and @wlemkows)

a discussion (no related file):
This PR is only about fixing existing tests.
New tests will be created to cover more complex test cases later.

utils/docker/run-build.sh line 34 at r8 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

not done...? I can still see a single commit :wink

Moved to #5572

utils/docker/run-build.sh line 33 at r12 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

... thay may hang if more jobs used

Moved to #5572

grom72

Reviewed 2 of 4 files at r14, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @lukaszstolarczuk and @wlemkows)

lukaszstolarczuk

Reviewed 1 of 6 files at r7, 4 of 4 files at r14, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @wlemkows)

Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>

grom72

Reviewed 2 of 2 files at r15, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @wlemkows)

grom72 force-pushed the obj-pmalloc-mt branch from ba54085 to 1585cba Compare March 31, 2023 18:58

wlemkows requested changes Apr 3, 2023

View reviewed changes

grom72 commented Apr 3, 2023

View reviewed changes

grom72 force-pushed the obj-pmalloc-mt branch 3 times, most recently from e98eb8f to a7dbdd5 Compare April 3, 2023 07:36

lukaszstolarczuk reviewed Apr 3, 2023

View reviewed changes

grom72 force-pushed the obj-pmalloc-mt branch from a7dbdd5 to 72f74ca Compare April 3, 2023 13:21

grom72 requested a review from wlemkows April 3, 2023 13:22

grom72 force-pushed the obj-pmalloc-mt branch from 72f74ca to 51ee751 Compare April 3, 2023 13:30

grom72 commented Apr 3, 2023

View reviewed changes

lukaszstolarczuk reviewed Apr 3, 2023

View reviewed changes

grom72 force-pushed the obj-pmalloc-mt branch 2 times, most recently from 880f9f8 to ead7c69 Compare April 3, 2023 14:11

grom72 commented Apr 3, 2023

View reviewed changes

lukaszstolarczuk reviewed Apr 3, 2023

View reviewed changes

grom72 force-pushed the obj-pmalloc-mt branch 2 times, most recently from 2fc7c14 to 5fc5283 Compare April 4, 2023 06:50

grom72 commented Apr 4, 2023

View reviewed changes

lukaszstolarczuk reviewed Apr 4, 2023

View reviewed changes

grom72 mentioned this pull request Apr 20, 2023

prealloc fails in heavy load conditions (obj_pmalloc_mt test fails) #5587

Open

test: verify prealloc result to avoid skipping wrong function execution

0fb9504

Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>

grom72 force-pushed the obj-pmalloc-mt branch from 5fc5283 to 0fb9504 Compare April 21, 2023 11:00

grom72 commented Apr 27, 2023

View reviewed changes

grom72 added this to the 1.14 milestone May 15, 2023

grom72 marked this pull request as draft June 27, 2023 10:49

janekmi modified the milestones: 2.0.0, 2.0.1 Jul 21, 2023

janekmi modified the milestones: 2.0.1, 2.0.2 Nov 23, 2023

janekmi modified the milestones: 2.1.0, 2.x Feb 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: verify prealloc result to avoid skipping wrong function execution #5568

test: verify prealloc result to avoid skipping wrong function execution #5568

grom72 commented Mar 31, 2023 •

edited by pbalcer

Loading

wlemkows Apr 3, 2023

grom72 Apr 4, 2023

wlemkows Apr 3, 2023

wlemkows Apr 3, 2023

grom72 Apr 4, 2023

wlemkows Apr 3, 2023

grom72 Apr 4, 2023

grom72 left a comment

codecov bot commented Apr 3, 2023 •

edited

Loading

lukaszstolarczuk left a comment

grom72 left a comment

grom72 left a comment

grom72 left a comment

lukaszstolarczuk left a comment

grom72 left a comment

grom72 left a comment

lukaszstolarczuk left a comment

lukaszstolarczuk left a comment

grom72 left a comment

grom72 left a comment

lukaszstolarczuk left a comment

grom72 left a comment

test: verify prealloc result to avoid skipping wrong function execution #5568

Are you sure you want to change the base?

test: verify prealloc result to avoid skipping wrong function execution #5568

Conversation

grom72 commented Mar 31, 2023 • edited by pbalcer Loading

wlemkows Apr 3, 2023

Choose a reason for hiding this comment

grom72 Apr 4, 2023

Choose a reason for hiding this comment

wlemkows Apr 3, 2023

Choose a reason for hiding this comment

wlemkows Apr 3, 2023

Choose a reason for hiding this comment

grom72 Apr 4, 2023

Choose a reason for hiding this comment

wlemkows Apr 3, 2023

Choose a reason for hiding this comment

grom72 Apr 4, 2023

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

codecov bot commented Apr 3, 2023 • edited Loading

Codecov Report

lukaszstolarczuk left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

lukaszstolarczuk left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

lukaszstolarczuk left a comment

Choose a reason for hiding this comment

lukaszstolarczuk left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

lukaszstolarczuk left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

grom72 commented Mar 31, 2023 •

edited by pbalcer

Loading

codecov bot commented Apr 3, 2023 •

edited

Loading