Stack error reasons on block retrieving failure [JIRA: RCS-223]

[kuenishi]

No description provided.

[kuenishi]

I wonder returning whole reasons or just head of this, which is better ... riak_cs_get_fsm handles this Reason just for logging. So maybe this should be rather a list of errors than the head of errors.

[kuenishi]

I also wonder right way to stack all errors, to add source of errors like from local get or remote.

[shino]

TL;DR; In my humble opinion, return all stacked errors to caller is
simple answer unless clever triage-and-prioritization filter
function is implemented.

---

It's difficult question: which error is most significant ;)

There are three kinds of "get" in block server (excluding legacy
n_val_one=false branch.)

• [Lo] local get with N=one
• [La] local get with N=all
• [Ra] remote get with N=all (proxy get)

Possible usual error cases and triage of them:

1. insufficent_vnodes in Lo: not informative (e.g just single node down)
2. notfound in Lo: not much informative (e.g. replica dificit in first primary)
3. notfound in La when proxy-get enabled: probably not informative
   (e.g. not replicated yet)
4. notfound in La when proxy-get disabled: significant (e.g. block data loss)
5. notfound in Ra: significant (e.g. block data loss)

Some other considerations:

• timeout is more significant than disconnected.
• If insufficient_vnodes errors are resolved in retry intervals,
  they can be ignored because they are just temporary.

---

I also wonder right way to stack all errors, to add source of errors
like from local get or remote.

Adding classification sounds nice :) e.g.

{error, [{local_one, {error, {insufficient_vnodes,0,need,1}}},
         {local_all, {error, timeout}},
         {local_all, {error, disconnected}},
         {local_all, {error, disconnected}}]}

{error, [{local_one, {error, notfound}},
         {local_all, {error, notfound}},
         {remote_all, {error, timeout}}]}.

[kuenishi]

Added three indicator: local_get, local_quorum and remote_quorum . Ready again ^^;

[shino]

This branch indicates

• get object failed by notfound with default N (usually 3) and
• proxy get is disabled or not-used (because local cluster is origin).

There is no strong reason that retry make the operation succeed after this error.
Original "immediately fail" seems reasonable.

[kuenishi]

My intention was not to change original behaviour. If we change original behaviour, it'd be another pull request?

[shino]

The behaivior is changed.

Original will call RetryFun with the atom failure and original
do_get_block/11 was implemented as follows:

do_get_block(ReplyPid, _Bucket, _Key, _ClusterID, _UseProxyGet, _ProxyActive,
             UUID, BlockNumber, _RcPid, MaxRetries, NumRetries)
  when is_atom(NumRetries) orelse NumRetries > MaxRetries ->
    Sorry = {error, notfound},

then there is no retry, immediate failure.

[kuenishi]

Updated. Looks like some riak_tests passing.

[shino]

{local_quorum, notfound} is missing in the stack.

[kuenishi]

oops

[kuenishi]

"May the force-push be with you." "Use the force-push, Luke!"

[shino]

This PR improves visibility inside block object fetch failures 👍 🎢 📇

[shino]

@borshop merge

[Basho-JIRA]

For release note:

On retrieving blocks there is a complex logic to resolve blocks when GET is requested from client. First, CS tries to retrieve a block with n_val=1 and if it fails, retry will be done in n_val=3. If the block cannot be resolved locally and proxy_get is enabled, the system is configured with datacenter replication. Thus Riak CS tries to perform proxied get to remote site. These fallback and retry logic and complex and hard to trace, especially in a faulty or unstable situation. This change improves error tracing of the whole sequence described above and will help diagnose issues. Specifically, for each block, block server stacks all errors returned from Riak client and reports every error reason as well as the type of call in which the error occurred.
PR: #1177

_[posted via JIRA by Kota Uenishi]_