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: add an option to recalculate user stats #20853

Merged
merged 3 commits into from Apr 13, 2018

Conversation

theanalyst
Copy link
Member

This series of commits adds an option to radosgw-admin called recalculate-stats
which resets the user head object to stats summed up according to user.bucket
omap entries. We call sync stats after reseting the stats in rgw-admin so that
stats are in sync with the current time.

cls/user: Introducing reset user stats which rewrites the user.buckets header to
the sum of entries and sizes from bucket omap entries

Otherwise we'll call sync stats with an empty argument which results in creation
of an empty ".buckets" object

Fixes: http://tracker.ceph.com/issues/23322
Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>
@theanalyst theanalyst force-pushed the rgw/recalc-stats branch 2 times, most recently from 1551584 to 8dbd974 Compare March 12, 2018 22:10
@theanalyst
Copy link
Member Author

Another possibility was adding yet another flag at cls_user_set_buckets_info for reset and checking that before we decrement stats, but that is making the function more complicated than it already is.

Copy link
Member

@oritwas oritwas left a comment

Choose a reason for hiding this comment

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

look goods just cosmetics

@@ -3613,6 +3613,7 @@ class RGWRados : public AdminSocketHook
int fix_tail_obj_locator(const RGWBucketInfo& bucket_info, rgw_obj_key& key, bool fix, bool *need_fix);

int cls_user_get_header(const string& user_id, cls_user_header *header);
int cls_user_clear_header(const string& user_id);
Copy link
Member

Choose a reason for hiding this comment

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

I prefer cls_user_recalc_stats or cls_user_reset_stats

Copy link
Member Author

Choose a reason for hiding this comment

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

sorry that was an initial method forgot while refactoring, fixing.

::cls_user_reset_stats(op);
r = ref.ioctx.operate(ref.oid, &op);

if (r < 0)
Copy link
Member

Choose a reason for hiding this comment

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

return r will be cleaner

cerr << "ERROR: could not clear user stats: " << cpp_strerror(-ret) << std::endl;
return -ret;
}
sync_stats = true;
Copy link
Member Author

Choose a reason for hiding this comment

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

@oritwas I can drop the sync_stats here, so the user can call it with --reset-stats --sync-stats if they need to explicitly sync stats or should we make sure that reset stats are also in sync with what cluster sees

Copy link
Member

Choose a reason for hiding this comment

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

you can drop it as you don't need it because you write the heade with the recalculated stats.

This is an implementation of reset user stats, that recalculates the user stats
purely based on the values of bucket entries in user.buckets object. This is
helpful in cases when user stats has been improperly set in case of manual
resharding etc.

Fixes: http://tracker.ceph.com/issues/23335
Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>
Adds a method in rgw-rados to reset user stats calling the earlier implemented
cls user reset stats.
In rgw-admin we add an option called --reset-stats that invokes this method.

Fixes: http://tracker.ceph.com/issues/23335
Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>
@theanalyst
Copy link
Member Author

Changelog:

@theanalyst
Copy link
Member Author

@oritwas do the updates look ok?

@smithfarm
Copy link
Contributor

@theanalyst @yuriw This PR is still open, yet its luminous backport [1] was merged?

[1] #21393

@theanalyst
Copy link
Member Author

we discussed this at standups yesterday and was given signal to merge

@theanalyst theanalyst merged commit 363c16b into ceph:master Apr 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants