[
https://issues.apache.org/jira/browse/HADOOP-16268?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16912498#comment-16912498
]
Erik Krogen commented on HADOOP-16268:
--------------------------------------
Great [~crh]! Thanks for the new changes. I have some small comments, mostly on
the tests.
* For the comment on {{IPC_CALLQUEUE_SERVER_FAILOVER_ENABLE}}, it seems that
there is already a block of similar configs above with a general comment
explaining how the namespacing works. If we push this key into that same block,
I think we can remove this comment and rely on that one?
* My IDE gives me a few warnings about {{testInsertionWithFailover}}:
** {{Exception}} is never thrown; you can remove the {{throws}}
** You can use diamond-typing for {{new FairCallQueue<>()}}
** {{p2}} is never used
* {{testInsertionWithFailover}} is great, but a bit long. Can we refactor a
method like:
{code}
private void addToQueueAndVerify(Schedulable call, int expectedQueue0, int
expectedQueue1, int expectedQueue2) {
Mockito.reset(fcq);
fcq.add(call);
Mockito.verify(fcq, times(expectedQueue0)).offerQueue(0, call);
Mockito.verify(fcq, times(expectedQueue1)).offerQueue(1, call);
Mockito.verify(fcq, times(expectedQueue2)).offerQueue(2, call);
}
{code}
> Allow custom wrapped exception to be thrown by server if RPC call queue is
> filled up
> ------------------------------------------------------------------------------------
>
> Key: HADOOP-16268
> URL: https://issues.apache.org/jira/browse/HADOOP-16268
> Project: Hadoop Common
> Issue Type: Improvement
> Reporter: CR Hota
> Assignee: CR Hota
> Priority: Major
> Attachments: HADOOP-16268.001.patch, HADOOP-16268.002.patch
>
>
> In the current implementation of callqueue manager,
> "CallQueueOverflowException" exceptions are always wrapping
> "RetriableException". Through configs servers should be allowed to throw
> custom exceptions based on new use cases.
> In CallQueueManager.java for backoff the below is done
> {code:java}
> // ideally this behavior should be controllable too.
> private void throwBackoff() throws IllegalStateException {
> throw CallQueueOverflowException.DISCONNECT;
> }
> {code}
> Since CallQueueOverflowException only wraps RetriableException clients would
> end up hitting the same server for retries. In use cases that router supports
> these overflowed requests could be handled by another router that shares the
> same state thus distributing load across a cluster of routers better. In the
> absence of any custom exception, current behavior should be supported.
> In CallQueueOverflowException class a new Standby exception wrap should be
> created. Something like the below
> {code:java}
> static final CallQueueOverflowException KEEPALIVE =
> new CallQueueOverflowException(
> new RetriableException(TOO_BUSY),
> RpcStatusProto.ERROR);
> static final CallQueueOverflowException DISCONNECT =
> new CallQueueOverflowException(
> new RetriableException(TOO_BUSY + " - disconnecting"),
> RpcStatusProto.FATAL);
> static final CallQueueOverflowException DISCONNECT2 =
> new CallQueueOverflowException(
> new StandbyException(TOO_BUSY + " - disconnecting"),
> RpcStatusProto.FATAL);
> {code}
>
--
This message was sent by Atlassian Jira
(v8.3.2#803003)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
