Skip to content

Sibling resolver [JIRA: RCS-249]#1188

Merged
borshop merged 3 commits into
developfrom
feature/cs-siblings-resolver
Sep 10, 2015
Merged

Sibling resolver [JIRA: RCS-249]#1188
borshop merged 3 commits into
developfrom
feature/cs-siblings-resolver

Conversation

@kuenishi

Copy link
Copy Markdown
Contributor

This is very well tested. Not really.

@kuenishi kuenishi added this to the 2.1.0 milestone Jul 15, 2015
@kuenishi

Copy link
Copy Markdown
Contributor Author

Good update of original script on timeout by Taka:

Fun = fun(Host, Port, B, K) ->
              {ok, Pid} = riakc_pb_socket:start_link(Host, Port),
              try
                  {ok, RiakObject} = riakc_pb_socket:get(Pid, B, K, [{r, all},{timeout, 60000*5}], 60000*5),
                  case lists:any(fun({_, <<>>}) -> true; ({MD, _}) -> dict:is_key(<<"X-Riak-Deleted">>, MD) =:= true end, riakc_obj:get_contents(RiakObject)) of
                      true ->
                          VClock = riakc_obj:vclock(RiakObject),
                          ok = riakc_pb_socket:delete_vclock(Pid, B, K, VClock);
                      false ->
                          {Meta, Block} = hd(riakc_obj:get_contents(RiakObject)),
                          RO1 = riakc_obj:update_metadata(RiakObject, Meta),
                          RO2 = riakc_obj:update_value(RO1, Block),
                          ok = riakc_pb_socket:put(Pid, RO2,[{timeout, 60000*5}],60000*5)
                  end
              after
                  riakc_pb_socket:stop(Pid)
              end
      end.

Comment thread src/riak_cs_console.erl Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's good to output time at start and end, which may help investigating timeout errors.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Another subtle parameter in resolving (possibly) large object is custom timeout value. Using large (like 10min) timeout value by default or add it as the fourth argument so that user can specify it.

@kuenishi

Copy link
Copy Markdown
Contributor Author

I would like to include Taka's awesome tool to this pull request. Pending to next sprint.

@Basho-JIRA Basho-JIRA changed the title Feature/cs siblings resolver Sibling resolver [JIRA: RCS-249] Jul 28, 2015
@kuenishi kuenishi modified the milestones: 2.2.0, 2.1.0 Jul 31, 2015
@kuenishi

kuenishi commented Sep 7, 2015

Copy link
Copy Markdown
Contributor Author

Updated this branch; I believe I've addressed all your comments ^^;.

Comment thread src/riak_cs_console.erl Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I prefer os:timestamp() unless monotonicity is needed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To align with timestamps in logs, I'd prefer calendar:universal_time(). thoughts?

@kuenishi

kuenishi commented Sep 8, 2015

Copy link
Copy Markdown
Contributor Author

Given that this function could be used via external scripts (not only console), I'll change all io:format to log output.

Comment thread src/riak_cs_console.erl Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

false in second element represents there are no siblings (single value), right?

@kuenishi kuenishi force-pushed the feature/cs-siblings-resolver branch from 51331d1 to c6249fa Compare September 8, 2015 05:29
* Add optional arguemnts on timeouts and put/get options
* Add logs to indicate start/end time of each resolution attempt
* Add logs to show numbers of siblings before trying to resolve them
* Add test case of siblings resolver and log output
@kuenishi kuenishi force-pushed the feature/cs-siblings-resolver branch from c6249fa to 53c0ac4 Compare September 8, 2015 05:41
@kuenishi

kuenishi commented Sep 8, 2015

Copy link
Copy Markdown
Contributor Author

force-pushed 👿

Comment thread src/riak_cs_console.erl Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This does not work for multibag environment because riak_cs_riak_client does not know which bag it should use for manifest.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Can we just take this function for single-bag cluster and tests?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, OK. Then I think three branches are not good because it wrongly looks like multibag ready. Just using master_pbc is sufficient.

Comment thread src/riak_cs_console.erl Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This log line is to indicate error occurrence. Is it better to include the word "error" (or "ERROR" or "fail" ...) for grep'pability?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think so, if that's error it should have log level as error but this is manual operation tool so I assume the operator's watching the log and see it. If needed, it should be grep'd with Not updating .

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it so hard to s/R/E/ ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No :)

@kuenishi

kuenishi commented Sep 8, 2015

Copy link
Copy Markdown
Contributor Author

Updated

@shino shino modified the milestones: 2.1.0, 2.2.0 Sep 9, 2015
@shino

shino commented Sep 10, 2015

Copy link
Copy Markdown
Contributor

Simple and effective 👍

borshop added a commit that referenced this pull request Sep 10, 2015
Sibling resolver [JIRA: RCS-249]

Reviewed-by: shino
@shino

shino commented Sep 10, 2015

Copy link
Copy Markdown
Contributor

@borshop merge

@borshop borshop merged commit 372e3bd into develop Sep 10, 2015
@kuenishi kuenishi deleted the feature/cs-siblings-resolver branch September 10, 2015 08:49
@Basho-JIRA

Copy link
Copy Markdown

For release note: Add a generic function for manual operations to resolve siblings of manifests and blocks

_[posted via JIRA by Kota Uenishi]_

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants