-
Notifications
You must be signed in to change notification settings - Fork 509
common: do not warn about explicity transaction abort #6117
Conversation
c45cfff to
7da208e
Compare
janekmi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 14 of 14 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @grom72 and @osalyk)
src/libpmemobj/tx.c line 947 at r1 (raw file):
if (user) { _CORE_LOG(CORE_LOG_LEVEL_HARK, errnum, "Explicit transaction abort: ");
Suggestion:
_CORE_LOG_W_ERRNO(CORE_LOG_LEVEL_HARK, "Explicit transaction abort");src/test/obj_basic_integration/TEST0 line 50 at r1 (raw file):
expect_normal_exit ./obj_basic_integration$EXESUFFIX $DIR/testfile1 if [ "$BUILD" = "debug" ]; then
Why is it limited to debug builds?
src/test/obj_basic_integration/TEST0 line 52 at r1 (raw file):
if [ "$BUILD" = "debug" ]; then grep "Explicit transaction abort" pmemobj$UNITTEST_NUM.log > pmemobjXXX.log mv pmemobjXXX.log pmemobj$UNITTEST_NUM.log
Suggestion:
grep "Explicit transaction abort" pmemobj$UNITTEST_NUM.log > grep$UNITTEST_NUM.log
src/test/obj_basic_integration/TEST0 line 53 at r1 (raw file):
grep "Explicit transaction abort" pmemobj$UNITTEST_NUM.log > pmemobjXXX.log mv pmemobjXXX.log pmemobj$UNITTEST_NUM.log cp pmemobj0-4.log.match pmemobj$UNITTEST_NUM.log.match
Please add the generated files (pmemobj0.log.match, ..., and pmemobj4.log.match) to local .gitignore.
Code quote:
pmemobj$UNITTEST_NUM.log.match
src/test/obj_basic_integration/TEST0 line 60 at r1 (raw file):
if [ "$BUILD" = "debug" ]; then rm pmemobj$UNITTEST_NUM.log.match fi
Maybe it does not have to be deleted assuming it is ignored by git? To be considered.
Code quote:
if [ "$BUILD" = "debug" ]; then
rm pmemobj$UNITTEST_NUM.log.match
fi
src/test/obj_basic_integration/TEST1 line 52 at r1 (raw file):
mv pmemobjXXX.log pmemobj$UNITTEST_NUM.log cp pmemobj0-4.log.match pmemobj$UNITTEST_NUM.log.match fi
Please create a common.sh file for this group of tests so you don't have to copy this bit over.
Code quote:
if [ "$BUILD" = "debug" ]; then
grep "Explicit transaction abort" pmemobj$UNITTEST_NUM.log > pmemobjXXX.log
mv pmemobjXXX.log pmemobj$UNITTEST_NUM.log
cp pmemobj0-4.log.match pmemobj$UNITTEST_NUM.log.match
fi
9dbfad7 to
be0e348
Compare
grom72
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 7 of 20 files reviewed, 6 unresolved discussions (waiting on @janekmi and @osalyk)
src/libpmemobj/tx.c line 947 at r1 (raw file):
if (user) { _CORE_LOG(CORE_LOG_LEVEL_HARK, errnum, "Explicit transaction abort: ");
Done.
src/test/obj_basic_integration/TEST0 line 50 at r1 (raw file):
Previously, janekmi (Jan Michalski) wrote…
Why is it limited to debug builds?
Done.
src/test/obj_basic_integration/TEST0 line 52 at r1 (raw file):
if [ "$BUILD" = "debug" ]; then grep "Explicit transaction abort" pmemobj$UNITTEST_NUM.log > pmemobjXXX.log mv pmemobjXXX.log pmemobj$UNITTEST_NUM.log
Done.
src/test/obj_basic_integration/TEST0 line 53 at r1 (raw file):
Previously, janekmi (Jan Michalski) wrote…
Please add the generated files (pmemobj0.log.match, ..., and pmemobj4.log.match) to local
.gitignore.
no
src/test/obj_basic_integration/TEST0 line 60 at r1 (raw file):
Previously, janekmi (Jan Michalski) wrote…
Maybe it does not have to be deleted assuming it is ignored by git? To be considered.
Done.
src/test/obj_basic_integration/TEST1 line 52 at r1 (raw file):
Previously, janekmi (Jan Michalski) wrote…
Please create a
common.shfile for this group of tests so you don't have to copy this bit over.
not needed
Use CORE_LOG_HARK instead. Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>
janekmi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 13 of 13 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72 and @osalyk)
src/test/obj_basic_integration/TEST0 line 40 at r3 (raw file):
unset PMEMOBJ_LOG_FILE unset PMEMOBJ_LOG_LEVEL
Since you set a custom logging function I suspect these two may not be necessary.
ddc54e5 to
b0a49ba
Compare
grom72
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 14 files at r1, 1 of 1 files at r2, 6 of 13 files at r3, 11 of 11 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @janekmi and @osalyk)
src/test/obj_basic_integration/TEST0 line 40 at r3 (raw file):
Previously, janekmi (Jan Michalski) wrote…
Since you set a custom logging function I suspect these two may not be necessary.
Done.
janekmi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 5 of 11 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72 and @osalyk)
src/test/obj_basic_integration/obj_basic_integration.c line 648 at r4 (raw file):
if ((argc == 3) && 0 == strcmp("log", argv[2])) { pmemobj_log_set_function(test_core_log_function); }
Why does it have to be conditional?
Code quote:
if ((argc == 3) && 0 == strcmp("log", argv[2])) {
pmemobj_log_set_function(test_core_log_function);
}
grom72
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @janekmi and @osalyk)
src/test/obj_basic_integration/obj_basic_integration.c line 648 at r4 (raw file):
Previously, janekmi (Jan Michalski) wrote…
Why does it have to be conditional?
To avoid garbage on the screen
We do not use log in TEST5-13.
osalyk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 14 files at r1, 1 of 1 files at r2, 7 of 13 files at r3, 11 of 11 files at r4, 3 of 3 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @janekmi)
Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>
grom72
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 13 files at r3, 3 of 3 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @janekmi)
531636a to
77a7c2f
Compare
grom72
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 3 files at r6, 3 of 3 files at r7, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @janekmi)
janekmi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 6 of 11 files at r4, 3 of 3 files at r7, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @grom72)
Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>
Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>
Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>
Fixes: #6107
This change is