rgw: admin api - add ability to sync user stats from admin api #17589
Conversation
There was a problem hiding this comment.
@nathanejohnson Your commit is not signed-off. Please do it.
nathanejohnson
force-pushed
the
feature/20883-adminapi_sync_stats_master
branch
from
d38b24f
to
11fbde7
Compare
|
@joscollin Sorry about that, I amended the commit message. Let me know if this is sufficient. Thank you |
Fixes: http://tracker.ceph.com/issues/21301 Signed-off-by: Nathan Johnson <nathan@nathanjohnson.org>
nathanejohnson
force-pushed
the
feature/20883-adminapi_sync_stats_master
branch
from
11fbde7
to
828412d
Compare
|
@joscollin I have tagged my commit. By the way, with the tags do you differentiate between a bug fix versus a new feature with the tags, or do they all prefix with Fixes: ? So regarding #5 in the guide, there is no documentation that I could find for the stats flag, but I would be happy to add it if you could point me in the right direction. |
|
@nathanejohnson It appears now, sorry. |
|
@joscollin regarding documentation of the sync parameter, should I add that to this PR? I can document the stats flag as well, but would that be more appropriate in a separate PR? Thanks |
|
@nathanejohnson It would be appropriate in a separate PR, which mentions this PR. |
| @@ -47,8 +48,11 @@ void RGWOp_User_Info::execute() | |||
|
|
|||
| RESTArgs::get_bool(s, "stats", false, &fetch_stats); | |||
|
|
|||
| RESTArgs::get_bool(s, "sync", false, &sync_stats); | |||
|
|
|||
There was a problem hiding this comment.
--sync-stats seems already exists in rgw_admin. So could you please clarify how do you use the command now ?
The tracker shows: $ radosgw-admin user stats --uid=<uid> --sync-stats
There was a problem hiding this comment.
If you call this command without --sync-stats , the stats may be stale. I noticed this when running some of the integration tests on an admin api client library I wrote. i.e., I could copy a file to a bucket, but stats would return with all zeros. All this PR would do is expose that same functionality via the admin API.
There was a problem hiding this comment.
Looks good. But I would request a review from @yehudasa too.
|
@nathanejohnson What about the |
|
@joscollin I'll get to those asap |
|
@joscollin results look clean modulo known valgrind issues and apparently unrelated ovh issues |
This adds the ability to trigger synchronizing user stats from the admin api.
http://tracker.ceph.com/issues/21301