Skip to content
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

rgw/dbstore: hide dbstore_log.h from rgw_main.cc #44581

Merged
merged 1 commit into from Jan 18, 2022

Conversation

cbodley
Copy link
Contributor

@cbodley cbodley commented Jan 13, 2022

dbstore_log.h sets global dout_subsys/dout_prefix macros, and was leaking into rgw_main.cc through the common/dbstore.h. this caused all of rgw_main's log output to start with the wrong prefix "rgw dbstore: "

Fixes: https://tracker.ceph.com/issues/53177

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

Copy link
Contributor

@soumyakoduri soumyakoduri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this!

@@ -33,15 +33,6 @@ class DBStoreManager {
cct = _cct;
default_db = createDB(default_tenant);
};
DBStoreManager(string logfile, int loglevel): DBStoreHandles() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is currently used at "https://github.com/ceph/ceph/blob/master/src/rgw/store/dbstore/dbstore_main.cc#L148" . That test file may soon be obsolete. So I am okay if it is removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, i missed that! i added the constructor back, but replaced dout_subsys with ceph_subsys_rgw

that constructor should go away, because the CephContext should be managed outside of the store. and note that global_init() returns a reference-counted boost::intrusive_ptr<CephContext>, but this class only stores a raw pointer to that - which means the intrusive_ptr would go out of scope and delete the CephContext right after global_init() returns

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah okay. Thanks! I will address it along with the other cleanup you have mentioned in the tracker.

dbstore_log.h sets global dout_subsys/dout_prefix macros, and was
leaking into rgw_main.cc through the common/dbstore.h. this caused all
of rgw_main's log output to start with the wrong prefix "rgw dbstore: "

Fixes: https://tracker.ceph.com/issues/53177

Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley cbodley added the needs-quincy-backport backport required for quincy label Jan 18, 2022
@cbodley cbodley merged commit 82db5e0 into ceph:master Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup needs-quincy-backport backport required for quincy rgw
Projects
None yet
2 participants