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: format logs in file rgw_lc.cc #19615

Merged
merged 1 commit into from Jan 10, 2018

Conversation

Projects
None yet
6 participants
@qrGitHub
Copy link

commented Dec 20, 2017

No description provided.

@amitkumar50
Copy link
Contributor

left a comment

LGTM

@cbodley

This comment has been minimized.

Copy link
Contributor

commented Dec 20, 2017

jenkins test docs

@qrGitHub

This comment has been minimized.

Copy link
Author

commented Dec 23, 2017

Hi @cbodley, can this be merged?

@qrGitHub

This comment has been minimized.

Copy link
Author

commented Dec 25, 2017

Hi @joscollin, would you help to merge this? Thanks.

@joscollin

This comment has been minimized.

Copy link
Member

commented Dec 26, 2017

jenkins test docs

@@ -351,13 +351,13 @@ int RGWLC::bucket_lc_process(string& shard_id)
string bucket_id = result[2];
int ret = store->get_bucket_info(obj_ctx, bucket_tenant, bucket_name, bucket_info, NULL, &bucket_attrs);
if (ret < 0) {
ldout(cct, 0) << "LC:get_bucket_info failed" << bucket_name <<dendl;
ldout(cct, 0) << "LC:get_bucket_info for " << bucket_name << " failed" << dendl;
return ret;
}

ret = bucket_info.bucket.bucket_id.compare(bucket_id) ;
if (ret !=0) {

This comment has been minimized.

Copy link
@joscollin

joscollin Dec 26, 2017

Member

put space after the !=

return ret;
}

ret = bucket_info.bucket.bucket_id.compare(bucket_id) ;
if (ret !=0) {
ldout(cct, 0) << "LC:old bucket id find, should be delete" << bucket_name <<dendl;
ldout(cct, 0) << "LC:old bucket id find, " << bucket_name << " should be deleted" << dendl;

This comment has been minimized.

Copy link
@joscollin

joscollin Dec 26, 2017

Member

I think this is better:
LC:old bucket id found. " << bucket_name << " should be deleted.

@@ -709,7 +709,8 @@ int RGWLC::process(int index, int max_lock_secs)
entry.second = lc_processing;
ret = cls_rgw_lc_set_entry(store->lc_pool_ctx, obj_names[index], entry);
if (ret < 0) {
dout(0) << "RGWLC::process() failed to set obj entry " << obj_names[index] << entry.first << entry.second << dendl;
dout(0) << "RGWLC::process() failed to set obj entry " << obj_names[index] <<
"(" << entry.first << "," << entry.second << ")" << dendl;

This comment has been minimized.

Copy link
@joscollin

joscollin Dec 26, 2017

Member

Put space before (

@qrGitHub qrGitHub force-pushed the qrGitHub:wip-rgw-log-adjustment2 branch from ef05ae5 to ae9aa41 Dec 26, 2017

@qrGitHub

This comment has been minimized.

Copy link
Author

commented Dec 26, 2017

@joscollin, changes are completed.

if (ret !=0) {
ldout(cct, 0) << "LC:old bucket id find, should be delete" << bucket_name <<dendl;
if (ret != 0) {
ldout(cct, 0) << "LC:old bucket id found. " << bucket_name << " should be deleted" << dendl;

This comment has been minimized.

Copy link
@joscollin

joscollin Dec 26, 2017

Member

This check should be if (ret == 0) in-order to find an old bucket id, right ?
@cbodley Could you please re-check this ?

This comment has been minimized.

Copy link
@qrGitHub

qrGitHub Dec 26, 2017

Author

No, the check is ok.

The compare here is to verify the 'bucket_id' in debug.rgw.log/lc. is valid.
Failed compare means the 'bucket_id' in debug.rgw.log/lc. is invalid, when we shouldn't run lifecycle for this bucket.
One possible circumstance is the bucket whose id is 'bucket_id' was removed, after which a new bucket with the same name is created.

Perhaps this line is better:
ldout(cct, 0) << "LC:old bucket id found. " << bucket_name << " has been deleted and re-created" << dendl;

This comment has been minimized.

Copy link
@joscollin

joscollin Dec 27, 2017

Member

@qrGitHub ok, thanks for clarifying.

@@ -683,7 +683,7 @@ int RGWLC::process(int index, int max_lock_secs)
cls_rgw_lc_obj_head head;
ret = cls_rgw_lc_get_head(store->lc_pool_ctx, obj_names[index], head);
if (ret < 0) {
dout(0) << "RGWLC::process() failed to get obj head " << obj_names[index] << ret << dendl;
dout(0) << "RGWLC::process() failed to get obj head " << obj_names[index] << ", ret=" << ret << dendl;

This comment has been minimized.

Copy link
@theanalyst

theanalyst Dec 27, 2017

Member

since we're touching the logs anyway, let's convert all dout calls to ldout, we already have a ctx

This comment has been minimized.

Copy link
@qrGitHub

qrGitHub Dec 27, 2017

Author

@theanalyst Ok, I will take this later. Would you tell me the difference between dout and ldout? Thanks.

Bingyin Zhang
rgw: format logs in file rgw_lc.cc
Signed-off-by: Bingyin Zhang <zhangbingyin@cloudin.cn>

@qrGitHub qrGitHub force-pushed the qrGitHub:wip-rgw-log-adjustment2 branch from ae9aa41 to b761887 Dec 28, 2017

@qrGitHub

This comment has been minimized.

Copy link
Author

commented Dec 28, 2017

change dout to ldout

@qrGitHub

This comment has been minimized.

Copy link
Author

commented Jan 8, 2018

Hi @cbodley @adamemerson, can this be merged? Thanks.

@yuriw

This comment has been minimized.

Copy link
Contributor

commented Jan 9, 2018

@yuriw yuriw merged commit 42c2155 into ceph:master Jan 10, 2018

5 checks passed

Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.