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.

Sign up for GitHub

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

Jump to bottom

Specify start/end timestamp in riak-cs-admin gc command #1147

Merged
merged 7 commits into from
May 26, 2015
Merged

Specify start/end timestamp in riak-cs-admin gc command #1147

merged 7 commits into from
May 26, 2015

Conversation

kuenishi
Copy link
Contributor

Several options (--start, --end, --leeway, --max_workers) are added
to riak-cs-admin gc batch command for deterministic GC batch
execution. ISO8601 type timestamp are supported at start/end option.
Leeway periods are still just by seconds.
Also, number of concurrency can be specified at each batch execution.
Although they are still optional, old riak-cs-admin batch <leeway>
style is also supported.

@kuenishi kuenishi added this to the 2.1.0 milestone May 18, 2015
@kuenishi
Specify start/end timestamp in riak-cs-admin gc command
356e115 
Several options (--start, --end, --leeway, --max_workers) are added
to `riak-cs-admin gc batch` command for deterministic GC batch
execution. ISO8601 type timestamp are supported at start/end option.
Leeway periods are still just by seconds.
Also, number of concurrency can be specified at each batch execution.
Although they are still optional, old `riak-cs-admin batch <leeway>`
style is also supported.
@kuenishi kuenishi changed the title [wip] Specify start/end timestamp in riak-cs-admin gc command Specify start/end timestamp in riak-cs-admin gc command May 20, 2015
@kuenishi
Copy link
Contributor Author

Added riak_test, removed [wip] tag and ready for review. This time it's working. After this get merged, block leak demo code will be another pull request with some updates.

shino
shino reviewed May 21, 2015
View reviewed changes
riak_test/src/rtcs.erl
@@ -1191,3 +1191,18 @@ get_cmd_result(Port, WaitTime) ->
after WaitTime ->
{error, timeout}
end.

%% Copy from rts:iso8601/1
Copy link
Contributor

Choose a reason for hiding this comment

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

No indent needed here.

@shino
Copy link
Contributor

shino commented May 21, 2015

Maybe a bug in the existing code, negative leeway can be specified.

% dev/dev1/bin/riak-cs-admin gc batch -10000 

18:16:50.455 [info] Starting garbage collection: leeway=-10000, batch_start=1432199810, max_workers=5

In getopt mode, it is propery rejected.

% dev/dev1/bin/riak-cs-admin gc batch --leeway=-10000                                                                   riak_cs-2.1/. [git:feature/gc-start-end-key] - 0
Usage: riak-cs-admin gc [-l <leeway>] [-s <start>] [-e <end>]
                        [-c <max_workers>]

  -l, --leeway       Leeway Seconds
  -s, --start        Start time (iso8601, )
  -e, --end          End time (iso8601, )
  -c, --max_workers  Number of concurrent worker

Invalid argument

shino
shino reviewed May 21, 2015
View reviewed changes
src/riak_cs_gc_console.erl
end, Options).

-spec datetime(string()) -> non_neg_integer().
datetime(S) ->
Copy link
Contributor

Choose a reason for hiding this comment

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

This function returns epoch, not datetime.

@kuenishi
Fix on comments
a631be2 
* change max_workers to max-workers as option name
* change gc_tests to use proper timeslices
* fix eunit
* strip some unused code
@kuenishi
Copy link
Contributor Author

I believe I've covered all your comments 🍣

@shino
Copy link
Contributor

shino commented May 25, 2015

Negative leeway (commented at #1147 (comment) ) is still accepted.

shino
shino reviewed May 25, 2015
View reviewed changes
riak_test/tests/gc_tests.erl
lager:debug("Keys: ~p", [Keys]),
StartKey = list_to_binary(integer_to_list(Start0)),
EndKey = list_to_binary(integer_to_list(End0)),
EndKeyHPF = fun(Key) when EndKey < Key -> true; (_Key) -> false end,
Copy link
Contributor

Choose a reason for hiding this comment

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

To be a little simpler, fun(Key) -> EndKey < Key end ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're a genius!

shino
shino reviewed May 26, 2015
View reviewed changes
test/riak_cs_gc_single_run_eqc.erl
%% {StartKey, _} = hd(SortedKeys),
%% {EndKey, _} = lists:last(SortedKeys),
BatchStart = riak_cs_gc:timestamp(),
%% ?debugVal({StartKey, EndKey, BatchStart}),
Copy link
Contributor

Choose a reason for hiding this comment

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

These four comments seems unnecessary?

@shino
Copy link
Contributor

shino commented May 26, 2015

eunit passed and all r_t succeeded both for basic and mb 🐉

borshop added a commit that referenced this pull request May 26, 2015
@borshop
Merge pull request #1147 from basho/feature/gc-start-end-key
49669e5 
Specify start/end timestamp in riak-cs-admin gc command

Reviewed-by: shino
@kuenishi
Copy link
Contributor Author

@borshop merge

@borshop borshop merged commit ffa5467 into develop May 26, 2015
@kuenishi kuenishi deleted the feature/gc-start-end-key branch May 26, 2015 04:13
@Basho-JIRA
Copy link

Release note: Add options to riak-cs-admin gc batch command, to specify start and end in manual batch execution. The --start option is equal to specifying epoch_start in advanced.config and could be replaced.

Details and usage is already written: https://github.com/basho/riak_cs/blob/develop/RELEASE-NOTES.md#new-riak-cs-admin-gc-command

_[posted via JIRA by Kota Uenishi]_

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@kuenishi kuenishi

Labels
JIRA: Closed JIRA: Done
Projects
None yet
Milestone
2.1.0
Development

Successfully merging this pull request may close these issues.
4 participants
@kuenishi @shino @Basho-JIRA @borshop