[
https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240151#comment-13240151
]
[email protected] commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6467
-----------------------------------------------------------
Changes seem to be coming together well. Thanks for the prompt turnaround. Some
feedback follows.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment14090>
Please add it to the top javadoc with explanation of how this property
works. Also, perhaps better called max-attempts.
In general, I suggest that these property names be defined at the top as
constants that are then referenced within the code. For example:
public static final String CONFIG_MAX_ATTEMPTS = "max-attempts";
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment14091>
formatting: } else { // same line
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment14092>
formatting: } else { // same line
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment14093>
Please log the exception.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment14097>
What is the reason for catching this exception differently? Seems redundant.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment14095>
Please log the exception. Ex:
logger.error("...", e2);
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment14096>
Looks like there is an off-by-one problem here. If the last try was
successful, the exception will be still raised.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment14099>
Please log the exception.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment14098>
This should be catching java.lang.Exception instead of FlumeException.
Otherwise you risk breaking the failover logic.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment14100>
log the exception.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment14102>
What is the purpose of this?
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java
<https://reviews.apache.org/r/4380/#comment14103>
Please log the exception.
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java
<https://reviews.apache.org/r/4380/#comment14104>
log the exception.
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java
<https://reviews.apache.org/r/4380/#comment14105>
log the exception.
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment14106>
Alternatively you could cast the client instance to RpcClient and invoke
the configure(properties) method on it directly.
- Arvind
On 2012-03-28 02:38:02, Hari Shreedharan wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4380/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-03-28 02:38:02)
bq.
bq.
bq. Review request for Flume.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Submitting an initial cut of FailoverRpcClient that uses the
NettyRpcClient under the hood. In this version, host selection is not exactly
the best, please make suggestions on how to improve it. As of now, the first
version will not have a backoff mechanism to not retry a host for a fixed time
etc(as discussed in the jira). I will add unit tests soon.
bq.
bq. Note that the actual "connect" call to a host is hidden from the
FailoverClient (by the Netty client or any other implementation, which we may
choose to use later). Since this connect call is hidden, failure to create a
client(the build function throwing an exception) is not being considered a
failure. Only a failure to append is considered a failure, and counted towards
the maximum number of tries. In other words, as far as the FailoverClient(for
that matter, any implementation of RpcClient interface) would consider an
append failure as failure, not a failure to a build() call - if we want to make
sure that a connect failure also is counted, we should move the connect call to
the append function and keep track of the connection state internally, and not
expect any code depending on an implementation of RpcClient(including other
clients which depend on pre-existing clients) to know that a build call also
creates a connection - this is exactly like a socket implementation, creating a
new socket does not initialize a connection, it is done explicitly.
bq.
bq.
bq. This addresses bug FLUME-962.
bq. https://issues.apache.org/jira/browse/FLUME-962
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java
965b2ff
bq. flume-ng-sdk/src/main/java/org/apache/flume/api/AbstractRpcClient.java
PRE-CREATION
bq. flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
PRE-CREATION
bq. flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClient.java a601213
bq. flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
351b5b1
bq. flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java
93bfee9
bq.
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java
PRE-CREATION
bq.
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java
a33e9c8
bq.
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java
0c94231
bq.
bq. Diff: https://reviews.apache.org/r/4380/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Unit tests added for the new functionality
bq.
bq.
bq. Thanks,
bq.
bq. Hari
bq.
bq.
> Failover capability for Client SDK
> ----------------------------------
>
> Key: FLUME-962
> URL: https://issues.apache.org/jira/browse/FLUME-962
> Project: Flume
> Issue Type: Sub-task
> Affects Versions: v1.0.0
> Reporter: Kathleen Ting
> Assignee: Hari Shreedharan
> Fix For: v1.2.0
>
> Attachments: FLUME-962-2.patch, FLUME-962-2.patch, FLUME-962-3.patch,
> FLUME-962-3.patch, FLUME-962-4.patch, FLUME-962-5.patch, FLUME-962-6.patch,
> FLUME-962-rebased-1.patch
>
>
> Need a client SDK for Flume that will allow clients to be able to failover
> from one source to another in case the first agent is not available. This
> will help in keeping client implementations developed outside of the project
> decoupled from internal details of HA implementation within Flume.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
