-
Notifications
You must be signed in to change notification settings - Fork 508
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
common: do not warn about explicity transaction abort #6117
Conversation
c45cfff
to
7da208e
Compare
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
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.sh
file 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 <[email protected]>
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
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.
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);
}
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.
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 <[email protected]>
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
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)
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 <[email protected]>
Signed-off-by: Tomasz Gromadzki <[email protected]>
Signed-off-by: Tomasz Gromadzki <[email protected]>
Fixes: #6107
This change is