You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by "Kathleen Ting (Created) (JIRA)" <ji...@apache.org> on 2012/02/11 00:42:59 UTC
[jira] [Created] (FLUME-962) Client SDK with failover capability
Client SDK with failover capability
-----------------------------------
Key: FLUME-962
URL: https://issues.apache.org/jira/browse/FLUME-962
Project: Flume
Issue Type: New Feature
Affects Versions: v1.0.0
Reporter: Kathleen Ting
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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240903#comment-13240903 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-29 01:39:04.251660)
Review request for Flume.
Changes
-------
Suggestions incorporated.
Summary
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-core/src/main/java/org/apache/flume/sink/AvroSink.java 3edc563
flume-ng-sdk/src/main/java/org/apache/flume/api/AbstractRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
flume-ng-core/src/main/java/org/apache/flume/client/avro/AvroCLIClient.java 704be7b
flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 5bc0472
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClient.java a601213
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 93bfee9
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java a33e9c8
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java 0c94231
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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, FLUME-962-rebased-4.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240151#comment-13240151 ]
jiraposter@reviews.apache.org 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231686#comment-13231686 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-16 22:21:59.443687)
Review request for Flume.
Changes
-------
Missing files and missing changes in last 2 diffs. This time all files and all changes are here!
Summary
-------
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 9497a3d
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Thanks,
Hari
> 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
> Fix For: v1.2.0
>
> Attachments: FLUME-962-2.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13238749#comment-13238749 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-26 20:00:38.286195)
Review request for Flume.
Changes
-------
Incorporated Mike's feedback.
Summary
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/main/java/org/apache/flume/api/ClientType.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 93bfee9
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java a33e9c8
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java 0c94231
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13237795#comment-13237795 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-25 08:04:09.704424)
Review request for Flume.
Changes
-------
Incorporated Mike's feedback. I feel it is not necessary to namespace and make the configuration properties too complex. We should not be making it too complex by making the syntax unnecessarily complex. Also I have not changed the reflection code logic in the factory, but made it more exception-safe. I cannot think of a way of writing a reflection-free, by still keeping it extensible(allowing more types of clients to be added). This is the standard technique used in most of the Flume factory classes.
Summary
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java a33e9c8
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java 0c94231
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 93bfee9
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClient.java a601213
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13232884#comment-13232884 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-19 20:43:56.971985)
Review request for Flume.
Changes
-------
Multithreading changes.
Summary
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 9497a3d
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
> 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
>
>
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240251#comment-13240251 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6476
-----------------------------------------------------------
Hari / Arvind,
I think we have made good progress with this patch. Also, the additional comments really help with readability, thanks for adding them.
However, the addition of the public configure() method on these objects reduces maintainability and API coherence. I agree that these implementation classes should read/handle their own configurations, however I think that would be better achieved via a public static inner Builder similar to the one in NettyAvroClient (the version that is currently checked in). We can make these configure() methods private and disallow anyone from calling them except the inner builder.
The problem with public configure() methods is that they require us to handle the case of in-flight reconfiguration. I also don't think the abstract class helps here because people can cast to (AbstractRpcClient) and call configure() if they want to because it's public. Although it should be frowned upon, the language allows it.
To instantiate and configure these objects in a way that enables maintainable factory methods, while not increasing the surface area or affecting the usability of the RpcClient API itself, I propose the following API design:
// inner builders in RpcClient implementation classes implement this interface
public interface RpcClientBuilder {
public RpcClient build(Properties config);
}
public NettyAvroRpcClient {
private NettyAvroRpcClient(...) { ... } // private constructor(s)
private configure(Properties config) { ... } // private configure() method
public static class Builder implements RpcClientBuilder { ... } // public builder or factory class
}
// something similar to the above for FailoverRpcClient as well
public RpcClientFactory {
private static enum ClientType {
SIMPLE(org.apache.flume.api.NettyAvroRpcClient.Builder.class),
FAILOVER(org.apache.flume.api.FailoverRpcClient.Builder.class),
OTHER(null);
// inner enum refers to the builders, not the implementations
private Class<? extends RpcClientBuilder> clientClass;
// ...
}
public RpcClient build(Properties config) {
// ... blah blah logic ...
return ClientType.SIMPLE.getClientClass().newInstance().build(config); // for example
}
}
The benefits of the above proposal are:
1. Implementation classes are totally fungible from an interface standpoint; casting doesn't buy you additional access to functionality (means we can switch them later and guarantee binary compat)
2. No need to maintain complicated state machines to determine when configure() can be called and what to do in response (since it's private, we can enforce that only the builder calls it)
3. RpcClient interface doesn't change (stays minimal); you can only append, check if it's active, and close it
4. RpcClientBuilder interface combined with using Class<...> in the enum allows us to (mostly) avoid reflection while also avoiding if-statements
Do you guys feel me on this? :)
If we can fix that, I think this patch is in decent enough shape to go in (modulo a couple formatting issues).
Regards,
Mike
- Mike
On 2012-03-28 05:22:34, 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 05:22:34)
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/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/NettyAvroRpcClient.java 965b2ff
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, FLUME-962-rebased-4.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
[jira] [Updated] (FLUME-962) Client SDK with failover capability
Posted by "Mike Percy (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mike Percy updated FLUME-962:
-----------------------------
Issue Type: Sub-task (was: New Feature)
Parent: FLUME-988
> Client SDK with failover capability
> -----------------------------------
>
> 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
>
> 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
[jira] [Updated] (FLUME-962) Failover capability for Client SDK
Posted by "Mike Percy (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mike Percy updated FLUME-962:
-----------------------------
Fix Version/s: v1.1.0
Summary: Failover capability for Client SDK (was: Client SDK with failover capability)
> 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
> Fix For: v1.1.0
>
>
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13239724#comment-13239724 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6375
-----------------------------------------------------------
Thanks for the patch Hari. Some feedback follows:
* General reminder that we need to follow the JCC style for formatting as specified in https://cwiki.apache.org/confluence/display/FLUME/How+to+Contribute#HowtoContribute-CodeQuality. Minimally, we need curly braces even if the code block is one line; else should be on the same line as the closing brace for the corresponding if statement; indentation should be aligned to the extent possible.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13845>
This is redundant
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13859>
The configuration specification should be in the class-level javadocs for clear visiblity.
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java
<https://reviews.apache.org/r/4380/#comment14042>
Exception should be logged.
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java
<https://reviews.apache.org/r/4380/#comment14044>
s/"Invalid port specified: " + hostAndProt[1]
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment13897>
Please use = instead of : to disambiguate since the value is also expected to use : char as a separator.
Also, it is better to point this javadoc to the RpcClient implementations so that they can be the one place where configuration is defined.
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment13907>
Please externalize this property in to a constants class or move it up top as a public static final String. Also suggest changing it to "clienttype" or "client.type".
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment13910>
This will certainly cause problems in environments that use a security manager.
Instead I suggest refactoring the client API to allow for a configure(Properties) contract via interface that is invoked directly. Other options can work too.
- Arvind
On 2012-03-26 20:00:38, 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-26 20:00:38)
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/ClientType.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/NettyAvroRpcClient.java 965b2ff
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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13233894#comment-13233894 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-20 22:39:17.101889)
Review request for Flume.
Changes
-------
Missed a bunch of files in the last diff.
Summary
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/main/java/org/apache/flume/api/ClientType.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 9497a3d
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java 8c40aa4
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java 0c94231
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
> 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
>
>
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13237413#comment-13237413 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6319
-----------------------------------------------------------
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13731>
Yes..missed this one when I made that change.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13732>
Agreed.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13733>
getClient logic is if the client itself is null it calls getNextClient(). This call is in effect made to getNextClient, which I felt should only be called through getClient() which is a synchronized wrapper
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13734>
agreed.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13735>
Until and unless we decide whether FlumeException should be checked or not, I don't think we should be throwing only EventDeliveryExceptions everywhere. This is not a delivery exception, this is a "client is closed exception", so I think this is the right exception to throw. Javadocs show that this can throw FlumeException too.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13736>
lastCheckedhost will be initialized at -1 i the updated patch, which will fix the calls issue. The logic is :
From current location go to end of loop, and then move forward until you hit current location again.
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment13730>
I didn't get this comment. We need the enum to figure out what client to build. We read the type from the properties and then create the Client instance using this. This helps make it extensible if we decide to add more RpcClients. So I can't see a way around it, other than a bunch of if-else-if.
- Hari
On 2012-03-22 03:43:56, 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-22 03:43:56)
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/ClientType.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/NettyAvroRpcClient.java 965b2ff
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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240255#comment-13240255 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
bq. On 2012-03-28 06:03:54, Arvind Prabhakar wrote:
bq. > Thanks for incorporating some of the previous feedback Hari.
bq. >
bq. > I did notice though that some of my feedback was not incorporated - which is OK as long as we discuss and agree to do so on the review. The best way to discuss this is by responding to individual comments with your feedback so that I know what to expect when you update the diffs. Without any comments from you, I expect that all of the feedback has been incorporated which is misleading. Conversely, if on every diff I have to do full review - then that poses a scalability challenge for the reviewer as you can see the number of diff increments that have gone into this issue alone.
bq. >
bq. > That said, I am putting marker comments on the items that were previously raised in the review and were not discussed or addressed. Please let me know your thoughts.
bq. >
bq. >
Interestingly, I had actually added a bunch of comments to your review. I will add them again here. Not sure why you cannot see them. Sorry for the inconvenience, I will try not to make the same mistake again.
bq. On 2012-03-28 06:03:54, Arvind Prabhakar wrote:
bq. > flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java, line 180
bq. > <https://reviews.apache.org/r/4380/diff/21/?file=97183#file97183line180>
bq. >
bq. > Previous request.
The only reason I did not add logs for these, is because they are not exactly error conditions, since the client is still ok, just one of the hosts just failed. I believe adding a logger.info might be ok.
bq. On 2012-03-28 06:03:54, Arvind Prabhakar wrote:
bq. > flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java, lines 287-299
bq. > <https://reviews.apache.org/r/4380/diff/21/?file=97183#file97183line287>
bq. >
bq. > Previous request.
I had added this in the previous review itself. Here it the reasoning. It is also mentioned in the code as a comment.
Basically the logic is explained above(Mike's comments). Anyway here it is:
We are currently connected to host number: i. i fails, we check all the hosts from i+1 to hosts.size()-1. If all fail, we check from host 0 to host i. Now, if none of these are available, throw exception. This is basically assuming that any host that fails might come back alive at some point in time. So we must go back and check every host and see if we can connect.
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6473
-----------------------------------------------------------
On 2012-03-28 05:22:34, 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 05:22:34)
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/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/NettyAvroRpcClient.java 965b2ff
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, FLUME-962-rebased-4.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231633#comment-13231633 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-16 21:11:18.242936)
Review request for Flume.
Changes
-------
Made some bug fix. Added unit tests.
Summary
-------
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 9497a3d
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Thanks,
Hari
> 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
> Fix For: v1.2.0
>
>
> 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
[jira] [Updated] (FLUME-962) Failover capability for Client SDK
Posted by "Hari Shreedharan (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hari Shreedharan updated FLUME-962:
-----------------------------------
Attachment: FLUME-962-2.patch
> 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
> Fix For: v1.2.0
>
> Attachments: FLUME-962-2.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
[jira] [Updated] (FLUME-962) Failover capability for Client SDK
Posted by "Hari Shreedharan (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hari Shreedharan updated FLUME-962:
-----------------------------------
Attachment: FLUME-962-6.patch
> 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
> 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
>
>
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240114#comment-13240114 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-28 02:15:38.128057)
Review request for Flume.
Changes
-------
Minor change in logic.
Summary
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClient.java a601213
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 93bfee9
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java a33e9c8
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java 0c94231
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13233300#comment-13233300 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-20 08:16:43.557937)
Review request for Flume.
Changes
-------
Incorporated much of Mike's feedback, the client factory now exposes only one additional (overloaded) method. I'd like some feedback on the properties object, whether I should use the same property name for all client types.
Summary
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 9497a3d
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java 8c40aa4
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java 0c94231
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
> 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
>
>
> 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
[jira] [Updated] (FLUME-962) Failover capability for Client SDK
Posted by "Hari Shreedharan (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hari Shreedharan updated FLUME-962:
-----------------------------------
Attachment: FLUME-962-3.patch
> 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
> Fix For: v1.2.0
>
> Attachments: FLUME-962-2.patch, FLUME-962-2.patch, FLUME-962-3.patch, FLUME-962-3.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
[jira] [Updated] (FLUME-962) Failover capability for Client SDK
Posted by "Arvind Prabhakar (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Arvind Prabhakar updated FLUME-962:
-----------------------------------
Resolution: Fixed
Status: Resolved (was: Patch Available)
Patch committed. Thanks Hari!
> 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, FLUME-962-rebased-4.patch, FLUME-992-final-2.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13235312#comment-13235312 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-22 03:43:56.608695)
Review request for Flume.
Changes
-------
Rebased on trunk. Sorry about some of the formatting only changes. I tried switching off those settings, and ended up screwing up the rest of the file.
Summary
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/main/java/org/apache/flume/api/ClientType.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 93bfee9
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java a33e9c8
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java 0c94231
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
> 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
>
>
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240120#comment-13240120 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-28 02:38:02.089560)
Review request for Flume.
Changes
-------
Did not want a configure function in the RpcClient interface. So added an AbstractRpcClient class, from which the Netty and Failover classes inherit. This class has the configure function. This allows us to configure the objects in the factory. Changing the interface or adding this would have broken bc, I felt this is safer, since we are not exposing the configure function any more.
Summary
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/AbstractRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClient.java a601213
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 93bfee9
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java a33e9c8
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java 0c94231
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240220#comment-13240220 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6473
-----------------------------------------------------------
Thanks for incorporating some of the previous feedback Hari.
I did notice though that some of my feedback was not incorporated - which is OK as long as we discuss and agree to do so on the review. The best way to discuss this is by responding to individual comments with your feedback so that I know what to expect when you update the diffs. Without any comments from you, I expect that all of the feedback has been incorporated which is misleading. Conversely, if on every diff I have to do full review - then that poses a scalability challenge for the reviewer as you can see the number of diff increments that have gone into this issue alone.
That said, I am putting marker comments on the items that were previously raised in the review and were not discussed or addressed. Please let me know your thoughts.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment14121>
Previous request.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment14122>
Previous request.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment14123>
Previous request.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment14124>
Previous request.
- Arvind
On 2012-03-28 05:22:34, 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 05:22:34)
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/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/NettyAvroRpcClient.java 965b2ff
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, FLUME-962-rebased-4.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
[jira] [Updated] (FLUME-962) Failover capability for Client SDK
Posted by "Hari Shreedharan (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hari Shreedharan updated FLUME-962:
-----------------------------------
Attachment: FLUME-992-final-2.patch
> 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, FLUME-962-rebased-4.patch, FLUME-992-final-2.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240927#comment-13240927 ]
Hudson commented on FLUME-962:
------------------------------
Integrated in flume-trunk #148 (See [https://builds.apache.org/job/flume-trunk/148/])
FLUME-962. Failover capability for Client SDK.
(Hari Shreedharan via Arvind Prabhakar) (Revision 1306687)
Result = SUCCESS
arvind : http://svn.apache.org/viewvc/?view=rev&rev=1306687
Files :
* /incubator/flume/trunk/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java
* /incubator/flume/trunk/flume-ng-core/src/main/java/org/apache/flume/client/avro/AvroCLIClient.java
* /incubator/flume/trunk/flume-ng-core/src/main/java/org/apache/flume/sink/AvroSink.java
* /incubator/flume/trunk/flume-ng-sdk/src/main/java/org/apache/flume/api/AbstractRpcClient.java
* /incubator/flume/trunk/flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
* /incubator/flume/trunk/flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java
* /incubator/flume/trunk/flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClient.java
* /incubator/flume/trunk/flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
* /incubator/flume/trunk/flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java
* /incubator/flume/trunk/flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java
* /incubator/flume/trunk/flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java
* /incubator/flume/trunk/flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java
> 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, FLUME-962-rebased-4.patch, FLUME-992-final-2.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "Hari Shreedharan (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13229836#comment-13229836 ]
Hari Shreedharan commented on FLUME-962:
----------------------------------------
I just wanted to clarify the requirements for this:
A) Allow the client code to specify more than one agents to connect to, perhaps in priority order.
B) Try to connect to highest priority host - if it works send data, else try next priority and so on.
C) In case of failure and the host no longer responds, then connect to highest priority host available.
The above implementation raises the following questions: If client is currently connected to priority A, and priority B comes online, where B has higher priority, should we simply continue on A unitl A fails or switch to B - This can cause high churn in connections and also will require us to have a thread keep track of which hosts are available(at least hosts with higher priority). Do we not have priorities at all? Take a set of hosts from the client, as backup to the one agent they want to connect to and connect to any one of those if available? In this case, we can have a thread which checks for our original agent, and sleeps for n seconds and checks again. The 2nd approach is simpler, we can even just allow the user to specify one failover agent, though the first one is more in line with our failover sink implmentation.
> 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
> Fix For: v1.2.0
>
>
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "Arvind Prabhakar (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13229907#comment-13229907 ]
Arvind Prabhakar commented on FLUME-962:
----------------------------------------
Sounds like a plan Hari. A couple of comments:
* The list of end points will be specified via configuration either looked up from a named file or passed as properties to the constructor/factory.
* I suggest not worrying about time based reconnect - but instead fail the operation if all the configured nodes have been tried and all have failed to accept the message.
> 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
> Fix For: v1.2.0
>
>
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231728#comment-13231728 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-16 23:13:38.692212)
Review request for Flume.
Changes
-------
Updated javadocs.
Summary
-------
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 9497a3d
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
> Fix For: v1.2.0
>
> Attachments: FLUME-962-2.patch, FLUME-962-2.patch, FLUME-962-3.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13232099#comment-13232099 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-17 23:23:15.402516)
Review request for Flume.
Changes
-------
Updating the description to explain why a client creation failure is not considered a failure, but only an append failure is considered a failure by the FailoverRpcClient.
Summary (updated)
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs
-----
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 9497a3d
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
> Fix For: v1.2.0
>
> Attachments: FLUME-962-2.patch, FLUME-962-2.patch, FLUME-962-3.patch, FLUME-962-3.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13233040#comment-13233040 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6089
-----------------------------------------------------------
Thanks for the patch!
I've added some feedback on the API inline in the review.
I mostly just have comments on the API, not on the failover implementation itself, since the API is harder to change later but the implementation should be straightforward to evolve over time.
I do have one general (moderately nitpicky) request: Can you please remove all of the "just spacing" changes? There are many places where the spacing was changed due to stylistic preference but no text was changed. I liked it the way I wrote it though. :) Unless we adopt a style standard other than the minimal one we have (Sun Java standard style, 2 spaces for indentation, 80 char max per line) then I don't think such changes should be done without a good reason. It's also worth noting that Netbeans generated the Apache license headers automatically. All that said, I'm not trying to discourage anyone from fixing things, shortening lines over 80 chars, adding to the javadocs, or starting a discussion about style standards. :)
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13106>
This should not be public. If it's public, people will cast from RpcClient to FailoverRpcClient to call it.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13107>
Missing @Override annotation
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13108>
Missing @Override annotation
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13109>
Missing @Override annotation
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13118>
client.isActive() does not imply that it's not closed. You have to clean up resources by closing it even if it's not active.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13119>
safe to call over and over again
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13120>
should probably happen outside the if block
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13110>
This should not be public
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13121>
This buildLock should not be necessary. What is static about it?
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java
<https://reviews.apache.org/r/4380/#comment13073>
This is not thread safe. Also not necessary, should be removed. We have to call close() on the transceiver to clean up resources.
Actually, isConnected() is a confusing name. All it means is that an Avro handshake has been completed at some point.
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment13082>
Maybe we should state that the failover is experimental and so the failover behavior policy is subject to change which is why it is not specified.
Also note that this public static factory interface should not specify the implementation class that is returned. Not even in the javadocs.
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment13080>
This should just be a getInstance() call with boolean failover argument = true
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment13081>
This should just be getInstance(Properties properties) with support for generating whatever is specified in the properties. User should not have to know there is a difference between the failover class and the composed class.
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment13078>
I don't think we should expose this API publicly. We want to minimize the surface area of this thing - there should be one right way to do things in this API and nothing else should be public.
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment13079>
This should not be exposed either.
- Mike
On 2012-03-19 20:43:56, 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-19 20:43:56)
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/FailoverRpcClient.java PRE-CREATION
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/RpcClientFactory.java 351b5b1
bq. flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 9497a3d
bq. flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
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
> 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
>
>
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240887#comment-13240887 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
bq. On 2012-03-29 01:09:32, Arvind Prabhakar wrote:
bq. > flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java, lines 93-94
bq. > <https://reviews.apache.org/r/4380/diff/22/?file=97201#file97201line93>
bq. >
bq. > Rename to getDefaultClientInstance(...)
bq.
bq. Hari Shreedharan wrote:
bq. This will require many other classes which call this to change. Should I go ahead and do that? Same for the other getInstance function.
We get back-compat with 1.1.0 if we leave it as-is. Arvind what problems does it cause to leave it as-is?
- Mike
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6499
-----------------------------------------------------------
On 2012-03-28 07:25:01, 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 07:25:01)
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/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/NettyAvroRpcClient.java 965b2ff
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, FLUME-962-rebased-4.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231744#comment-13231744 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-17 00:07:15.857976)
Review request for Flume.
Changes
-------
Updating some concurrency stuff.
Summary
-------
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 9497a3d
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
> Fix For: v1.2.0
>
> Attachments: FLUME-962-2.patch, FLUME-962-2.patch, FLUME-962-3.patch, FLUME-962-3.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13234014#comment-13234014 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-21 01:24:55.388648)
Review request for Flume.
Changes
-------
Changed the properties format, to make it more extensible.
Summary
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java 0c94231
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java 8c40aa4
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 9497a3d
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/ClientType.java PRE-CREATION
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
> 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
>
>
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240030#comment-13240030 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-27 22:54:28.198621)
Review request for Flume.
Changes
-------
Thanks for the feedback Arvind. I have incorporated all of them. In order to allow configuration through a public function which would be specified by the RpcClient Interface, I made the constructors protected - so that we can create the objects from the factory. I didn't want to make the build function public, so removed it, and created a public configure() function in the RpcClient Interface.
Summary
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClient.java a601213
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 93bfee9
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java a33e9c8
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java 0c94231
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240259#comment-13240259 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
bq. On 2012-03-28 06:03:54, Arvind Prabhakar wrote:
bq. > Thanks for incorporating some of the previous feedback Hari.
bq. >
bq. > I did notice though that some of my feedback was not incorporated - which is OK as long as we discuss and agree to do so on the review. The best way to discuss this is by responding to individual comments with your feedback so that I know what to expect when you update the diffs. Without any comments from you, I expect that all of the feedback has been incorporated which is misleading. Conversely, if on every diff I have to do full review - then that poses a scalability challenge for the reviewer as you can see the number of diff increments that have gone into this issue alone.
bq. >
bq. > That said, I am putting marker comments on the items that were previously raised in the review and were not discussed or addressed. Please let me know your thoughts.
bq. >
bq. >
bq.
bq. Hari Shreedharan wrote:
bq. Interestingly, I had actually added a bunch of comments to your review. I will add them again here. Not sure why you cannot see them. Sorry for the inconvenience, I will try not to make the same mistake again.
bq.
bq.
No problem Hari. I too have goofed up a few times where the review comments did not post correctly. Totally understand that.
bq. On 2012-03-28 06:03:54, Arvind Prabhakar wrote:
bq. > flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java, line 180
bq. > <https://reviews.apache.org/r/4380/diff/21/?file=97183#file97183line180>
bq. >
bq. > Previous request.
bq.
bq. Hari Shreedharan wrote:
bq. The only reason I did not add logs for these, is because they are not exactly error conditions, since the client is still ok, just one of the hosts just failed. I believe adding a logger.info might be ok.
I do believe these are errors that should be reported as such. Without the stack trace and other relevant information, it will be hard for the user to debug or fix the issue.
bq. On 2012-03-28 06:03:54, Arvind Prabhakar wrote:
bq. > flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java, lines 287-299
bq. > <https://reviews.apache.org/r/4380/diff/21/?file=97183#file97183line287>
bq. >
bq. > Previous request.
bq.
bq. Hari Shreedharan wrote:
bq. I had added this in the previous review itself. Here it the reasoning. It is also mentioned in the code as a comment.
bq.
bq. Basically the logic is explained above(Mike's comments). Anyway here it is:
bq.
bq. We are currently connected to host number: i. i fails, we check all the hosts from i+1 to hosts.size()-1. If all fail, we check from host 0 to host i. Now, if none of these are available, throw exception. This is basically assuming that any host that fails might come back alive at some point in time. So we must go back and check every host and see if we can connect.
Thanks for the explanation Hari, that makes sense.
- Arvind
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6473
-----------------------------------------------------------
On 2012-03-28 07:25:01, 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 07:25:01)
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/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/NettyAvroRpcClient.java 965b2ff
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, FLUME-962-rebased-4.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13237712#comment-13237712 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
bq. On 2012-03-24 00:47:27, Mike Percy wrote:
bq. > Hi, thanks for doing this work.
bq. >
bq. > I have the following suggestions on the config format:
bq. >
bq. > The properties format is not extensible. Right now it looks like this:
bq. > hosts = host1 host2
bq. > host1 = hostname1:port1
bq. > host2 = hostname2:port2
bq. >
bq. > How about something like:
bq. > hosts = host1 host2
bq. > host.host1.endpoint = hostname1:port1
bq. > host.host2.endpoint = hostname2:port2
bq. >
bq. > In this way, we namespace the host specifications, and also have the option of adding stuff like groups or priorities to them later if we want.
bq. >
bq. > Also, I have reservations about using the enum and reflection. My thoughts regarding that are below.
I don't think we should make the properties more and more complex. I already have reservations about taking in a properties file.
Namespacing should not be required if this is going to be kept simple. I know the client might have other properties data(if it is being loaded from a file, in that case namespacing it as just as "hosts" also can cause issues - we will need to make it something like flume.hosts or even org.apache.flume.hosts at which point we are overthinking it). Either way I would assume that the client code would be using some data of his own and creating this properties object and passing it to our code, rather than passing the config file entirely to our code anyway - open source aside, passing your config, (not pertinent to that component) is a bad idea.
I think this would also be extensible:
host1 = hostname1:port1
host2 = hostname2:port2
If we add groups we can do something like:
host1.group = <blah>
or
group1.hosts = host1, host2
same applies for priorities.
I think it is good to make the properties extensible, but making it more and more complex, expecting to add more features in the future is not something I want to do right now. I'd rather keep it simple, than make it complex when most of the features like groups and priorities(which also can be added as shown above) are not likely to be used very frequently.
For now, I think this should be kept simple.
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6312
-----------------------------------------------------------
On 2012-03-22 03:43:56, 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-22 03:43:56)
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/ClientType.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/NettyAvroRpcClient.java 965b2ff
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
[jira] [Assigned] (FLUME-962) Failover capability for Client SDK
Posted by "Arvind Prabhakar (Assigned) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Arvind Prabhakar reassigned FLUME-962:
--------------------------------------
Assignee: Hari Shreedharan
> 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
[jira] [Updated] (FLUME-962) Failover capability for Client SDK
Posted by "Hari Shreedharan (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hari Shreedharan updated FLUME-962:
-----------------------------------
Attachment: FLUME-962-3.patch
> 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
> Fix For: v1.2.0
>
> Attachments: FLUME-962-2.patch, FLUME-962-2.patch, FLUME-962-3.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240740#comment-13240740 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
bq. On 2012-03-28 07:15:54, Mike Percy wrote:
bq. > Hari / Arvind,
bq. > I think we have made good progress with this patch. Also, the additional comments really help with readability, thanks for adding them.
bq. >
bq. > However, the addition of the public configure() method on these objects reduces maintainability and API coherence. I agree that these implementation classes should read/handle their own configurations, however I think that would be better achieved via a public static inner Builder similar to the one in NettyAvroClient (the version that is currently checked in). We can make these configure() methods private and disallow anyone from calling them except the inner builder.
bq. >
bq. > The problem with public configure() methods is that they require us to handle the case of in-flight reconfiguration. I also don't think the abstract class helps here because people can cast to (AbstractRpcClient) and call configure() if they want to because it's public. Although it should be frowned upon, the language allows it.
bq. >
bq. > To instantiate and configure these objects in a way that enables maintainable factory methods, while not increasing the surface area or affecting the usability of the RpcClient API itself, I propose the following API design:
bq. >
bq. > // inner builders in RpcClient implementation classes implement this interface
bq. > public interface RpcClientBuilder {
bq. > public RpcClient build(Properties config);
bq. > }
bq. >
bq. > public NettyAvroRpcClient {
bq. > private NettyAvroRpcClient(...) { ... } // private constructor(s)
bq. > private configure(Properties config) { ... } // private configure() method
bq. > public static class Builder implements RpcClientBuilder { ... } // public builder or factory class
bq. > }
bq. >
bq. > // something similar to the above for FailoverRpcClient as well
bq. >
bq. > public RpcClientFactory {
bq. > private static enum ClientType {
bq. > SIMPLE(org.apache.flume.api.NettyAvroRpcClient.Builder.class),
bq. > FAILOVER(org.apache.flume.api.FailoverRpcClient.Builder.class),
bq. > OTHER(null);
bq. >
bq. > // inner enum refers to the builders, not the implementations
bq. > private Class<? extends RpcClientBuilder> clientClass;
bq. > // ...
bq. > }
bq. >
bq. > public RpcClient build(Properties config) {
bq. > // ... blah blah logic ...
bq. > return ClientType.SIMPLE.getClientClass().newInstance().build(config); // for example
bq. > }
bq. > }
bq. >
bq. > The benefits of the above proposal are:
bq. > 1. Implementation classes are totally fungible from an interface standpoint; casting doesn't buy you additional access to functionality (means we can switch them later and guarantee binary compat)
bq. > 2. No need to maintain complicated state machines to determine when configure() can be called and what to do in response (since it's private, we can enforce that only the builder calls it)
bq. > 3. RpcClient interface doesn't change (stays minimal); you can only append, check if it's active, and close it
bq. > 4. RpcClientBuilder interface combined with using Class<...> in the enum allows us to (mostly) avoid reflection while also avoiding if-statements
bq. >
bq. > Do you guys feel me on this? :)
bq. >
bq. > If we can fix that, I think this patch is in decent enough shape to go in (modulo a couple formatting issues).
bq. >
bq. > Regards,
bq. > Mike
bq. >
bq.
bq. Arvind Prabhakar wrote:
bq. I personally will be happy with default constructors and public configure() methods with work-arounds like raising exception on reconfiguration attempts etc. But yes, as an API we can and should do better. In that regard, I do see the merit in Mike's suggestion above.
bq.
bq. Hari, what do you think?
bq.
bq.
If you guys want to move forward with this patch then go ahead. We can follow up on this and possibly refactor it later.
Best,
Mike
- Mike
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6476
-----------------------------------------------------------
On 2012-03-28 07:25:01, 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 07:25:01)
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/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/NettyAvroRpcClient.java 965b2ff
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, FLUME-962-rebased-4.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240865#comment-13240865 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6499
-----------------------------------------------------------
Ship it!
+1. Thanks for incorporating the changes Hari. There is some suggestions below for you to consider.
Please attach the patch to the Jira.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment14159>
please use a public static final String CONFIG_HOSTS = "hosts".
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment14157>
Please make it public static final, and rename it CONF_CLIENT_TYPE instead.
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment14153>
Please reference the Enum directly.
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment14156>
Rename to getDefaultClientInstance(...)
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment14155>
Rename to getDefaultClientInstance(...)
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment14151>
This should be called DEFAULT
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment14152>
Perhaps better called DEFAULT_FAILOVER
- Arvind
On 2012-03-28 07:25:01, 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 07:25:01)
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/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/NettyAvroRpcClient.java 965b2ff
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, FLUME-962-rebased-4.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240256#comment-13240256 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-28 07:25:01.534978)
Review request for Flume.
Changes
-------
Incorporating Arvind's logging suggestions.
Summary
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/main/java/org/apache/flume/api/AbstractRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClient.java a601213
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 93bfee9
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java a33e9c8
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java 0c94231
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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, FLUME-962-rebased-4.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13237364#comment-13237364 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6312
-----------------------------------------------------------
Hi, thanks for doing this work.
I have the following suggestions on the config format:
The properties format is not extensible. Right now it looks like this:
hosts = host1 host2
host1 = hostname1:port1
host2 = hostname2:port2
How about something like:
hosts = host1 host2
host.host1.endpoint = hostname1:port1
host.host2.endpoint = hostname2:port2
In this way, we namespace the host specifications, and also have the option of adding stuff like groups or priorities to them later if we want.
Also, I have reservations about using the enum and reflection. My thoughts regarding that are below.
flume-ng-sdk/src/main/java/org/apache/flume/api/ClientType.java
<https://reviews.apache.org/r/4380/#comment13701>
Does it really matter that it's backed by Netty? Maybe we can call this SIMPLE instead.
flume-ng-sdk/src/main/java/org/apache/flume/api/ClientType.java
<https://reviews.apache.org/r/4380/#comment13702>
I think this should be declared final
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13729>
This should be private. There is a public accessor method for the corresponding field.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13719>
Shouldn't this start at -1 ?
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13711>
We are indexing into this variable in this class. One should only use a LinkedList if there is only sequential iteration happening. Since we are directly indexing into this variable we should be using an ArrayList instead.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13718>
Is this max total tries or max retries? There's a difference of one between them.
Also it might make sense to name this property MaxTries.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13714>
Why assign to client member variable when getClient() already assigned to it?
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13708>
Should only set this within a synchronized block if it is guarded by (this). A private synchronized setter would work.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13704>
We should not throw vanilla FlumeException. It should be EventDeliveryException only (per the RpcClient interface).
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13703>
Can be dangerous / misleading to name a local variable with the same name as a member variable. Consider renaming to curClient or something.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13705>
Should throw EventDeliveryException here.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13706>
We should not throw FlumeException from this method, per the RpcClient interface.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13715>
Masks member variable.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13707>
Throw EventDeliveryException
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13716>
while (tries < maxTries)
Consider using a for() loop instead of a while().
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13717>
if (tries == maxTries)
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13710>
masks member variable
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13709>
Why not while() { ... } ?
Generally, can the logic here be simplified? It took me a long time to trace through the code. It seems complex and is using several variables. Maybe base the logic on a variable called numTries, and loop while (numTries < hosts.length - 1). It's hard to follow if both count and limit are changed inside the loop like this.
Also, I think there is an off-by-one error here because if lastCheckedHost starts @ 0, and hosts.length == 3, then you will only call getInstance() twice before bailing out. But if lastCheckedHost == 2 when beginning, then getInstance() will be called 3 times.
One problem is that the case where we are trying for the first time is maybe not handled explicitly differently than the case where we are doing a retry. Might make sense to separate that logic out a little more.
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java
<https://reviews.apache.org/r/4380/#comment13728>
Might be a good idea to remove this method. Just use the Properties one.
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment13724>
Let's not document the default batchSize. They should use getBatchSize() to detect it.
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java
<https://reviews.apache.org/r/4380/#comment13727>
I'm not excited about using reflection here. We lose traceability in IDEs and compile-time type checking. Also, the enum is not even used directly it's just basically reflected too. So what's the benefit.
- Mike
On 2012-03-22 03:43:56, 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-22 03:43:56)
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/ClientType.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/NettyAvroRpcClient.java 965b2ff
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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240913#comment-13240913 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6506
-----------------------------------------------------------
Ship it!
+1
- Arvind
On 2012-03-29 01:39:04, 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-29 01:39:04)
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-core/src/main/java/org/apache/flume/sink/AvroSink.java 3edc563
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-core/src/main/java/org/apache/flume/client/avro/AvroCLIClient.java 704be7b
bq. flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 5bc0472
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/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, FLUME-962-rebased-4.patch, FLUME-992-final-2.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
[jira] [Updated] (FLUME-962) Failover capability for Client SDK
Posted by "Hari Shreedharan (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hari Shreedharan updated FLUME-962:
-----------------------------------
Attachment: FLUME-962-4.patch
> 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
> 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
>
>
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "Hari Shreedharan (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13229900#comment-13229900 ]
Hari Shreedharan commented on FLUME-962:
----------------------------------------
Arvind, Thanks. That is what I was planning to do. Take a list from the client and just try to send data over the first one. If it fails, move on to the next. Say agent n fails, then start searching from agent 1 to find if one will connect. Maybe we can check if we tried to connect to that host in the last k seconds(maybe the user can provide this?), before we try to connect to that host again?:
A) Take a list of nodes from the user, connect to node[0], if it fails, to node[1] etc. Basically if any node fails, start from node[0] and check if we can connect to any node, provided we did not try to connect, and fail in the last k seconds.
> 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
> Fix For: v1.2.0
>
>
> 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
[jira] [Updated] (FLUME-962) Failover capability for Client SDK
Posted by "Hari Shreedharan (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hari Shreedharan updated FLUME-962:
-----------------------------------
Status: Patch Available (was: Open)
> 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
> Fix For: v1.2.0
>
> Attachments: FLUME-962-2.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231670#comment-13231670 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-16 22:06:48.083581)
Review request for Flume.
Changes
-------
Seems like the new files didnt make it into the last diffs.
Summary
-------
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Thanks,
Hari
> 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
> Fix For: v1.2.0
>
> Attachments: FLUME-962-2.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
[jira] [Updated] (FLUME-962) Failover capability for Client SDK
Posted by "Hari Shreedharan (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hari Shreedharan updated FLUME-962:
-----------------------------------
Attachment: FLUME-962-5.patch
> 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
> 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
>
>
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240550#comment-13240550 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
bq. On 2012-03-28 07:15:54, Mike Percy wrote:
bq. > Hari / Arvind,
bq. > I think we have made good progress with this patch. Also, the additional comments really help with readability, thanks for adding them.
bq. >
bq. > However, the addition of the public configure() method on these objects reduces maintainability and API coherence. I agree that these implementation classes should read/handle their own configurations, however I think that would be better achieved via a public static inner Builder similar to the one in NettyAvroClient (the version that is currently checked in). We can make these configure() methods private and disallow anyone from calling them except the inner builder.
bq. >
bq. > The problem with public configure() methods is that they require us to handle the case of in-flight reconfiguration. I also don't think the abstract class helps here because people can cast to (AbstractRpcClient) and call configure() if they want to because it's public. Although it should be frowned upon, the language allows it.
bq. >
bq. > To instantiate and configure these objects in a way that enables maintainable factory methods, while not increasing the surface area or affecting the usability of the RpcClient API itself, I propose the following API design:
bq. >
bq. > // inner builders in RpcClient implementation classes implement this interface
bq. > public interface RpcClientBuilder {
bq. > public RpcClient build(Properties config);
bq. > }
bq. >
bq. > public NettyAvroRpcClient {
bq. > private NettyAvroRpcClient(...) { ... } // private constructor(s)
bq. > private configure(Properties config) { ... } // private configure() method
bq. > public static class Builder implements RpcClientBuilder { ... } // public builder or factory class
bq. > }
bq. >
bq. > // something similar to the above for FailoverRpcClient as well
bq. >
bq. > public RpcClientFactory {
bq. > private static enum ClientType {
bq. > SIMPLE(org.apache.flume.api.NettyAvroRpcClient.Builder.class),
bq. > FAILOVER(org.apache.flume.api.FailoverRpcClient.Builder.class),
bq. > OTHER(null);
bq. >
bq. > // inner enum refers to the builders, not the implementations
bq. > private Class<? extends RpcClientBuilder> clientClass;
bq. > // ...
bq. > }
bq. >
bq. > public RpcClient build(Properties config) {
bq. > // ... blah blah logic ...
bq. > return ClientType.SIMPLE.getClientClass().newInstance().build(config); // for example
bq. > }
bq. > }
bq. >
bq. > The benefits of the above proposal are:
bq. > 1. Implementation classes are totally fungible from an interface standpoint; casting doesn't buy you additional access to functionality (means we can switch them later and guarantee binary compat)
bq. > 2. No need to maintain complicated state machines to determine when configure() can be called and what to do in response (since it's private, we can enforce that only the builder calls it)
bq. > 3. RpcClient interface doesn't change (stays minimal); you can only append, check if it's active, and close it
bq. > 4. RpcClientBuilder interface combined with using Class<...> in the enum allows us to (mostly) avoid reflection while also avoiding if-statements
bq. >
bq. > Do you guys feel me on this? :)
bq. >
bq. > If we can fix that, I think this patch is in decent enough shape to go in (modulo a couple formatting issues).
bq. >
bq. > Regards,
bq. > Mike
bq. >
I personally will be happy with default constructors and public configure() methods with work-arounds like raising exception on reconfiguration attempts etc. But yes, as an API we can and should do better. In that regard, I do see the merit in Mike's suggestion above.
Hari, what do you think?
- Arvind
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6476
-----------------------------------------------------------
On 2012-03-28 07:25:01, 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 07:25:01)
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/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/NettyAvroRpcClient.java 965b2ff
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, FLUME-962-rebased-4.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240037#comment-13240037 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-27 23:03:49.296754)
Review request for Flume.
Changes
-------
Thanks for the feedback Arvind. I have incorporated all of them. In order to allow configuration through a public function which would be specified by the RpcClient Interface, I made the constructors protected - so that we can create the objects from the factory. I didn't want to make the build function public, so removed it, and created a public configure() function in the RpcClient Interface.
Summary
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClient.java a601213
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 93bfee9
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java a33e9c8
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java 0c94231
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "Arvind Prabhakar (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13229887#comment-13229887 ]
Arvind Prabhakar commented on FLUME-962:
----------------------------------------
@Hari - for starters let us keep things simple: Allow the specification of more than one end points in a pre-specified order. If the first fails, move on to the next and stay on the next for a specified amount of time. After the time has elapsed, try to revert back to the previous end point and so on.
In fact, for the first cut implementation we should go without the ability to dynamically revert to prior host based on time. We can factor those requirements when this feature gets used and adopted in the field.
> 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
> Fix For: v1.2.0
>
>
> 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
[jira] [Updated] (FLUME-962) Failover capability for Client SDK
Posted by "Hari Shreedharan (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hari Shreedharan updated FLUME-962:
-----------------------------------
Attachment: FLUME-962-rebased-4.patch
> 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, FLUME-962-rebased-4.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "Arvind Prabhakar (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13230372#comment-13230372 ]
Arvind Prabhakar commented on FLUME-962:
----------------------------------------
I do feel that configuration, ideally via both properties file and constructor - is the right way to go in order to make sure that the system is easily usable and manageable for building light-weight clients.
There is some discussion of relevance on FLUME-989 where the mention of configuration/properties is made that you should glance through.
> 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
> Fix For: v1.2.0
>
>
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240247#comment-13240247 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
bq. On 2012-03-28 04:17:07, Arvind Prabhakar wrote:
bq. > flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java, lines 292-302
bq. > <https://reviews.apache.org/r/4380/diff/19/?file=97138#file97138line292>
bq. >
bq. > What is the purpose of this?
Basically the logic is explained above(Mike's comments). Anyway here it is:
We are currently connected to host number: i. i fails, we check all the hosts from i+1 to hosts.size()-1. If all fail, we check from host 0 to host i. Now, if none of these are available, throw exception. This is basically assuming that any host that fails might come back alive at some point in time. So we must go back and check every host and see if we can connect.
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6467
-----------------------------------------------------------
On 2012-03-28 05:22:34, 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 05:22:34)
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/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/NettyAvroRpcClient.java 965b2ff
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, FLUME-962-rebased-4.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13230964#comment-13230964 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
Review request for Flume.
Summary
-------
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs
-----
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Thanks,
Hari
> 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
> Fix For: v1.2.0
>
>
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13234022#comment-13234022 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-21 01:37:21.662817)
Review request for Flume.
Changes
-------
Javadocs update
Summary
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java 0c94231
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java 8c40aa4
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 9497a3d
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/ClientType.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
> 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
>
>
> 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
[jira] [Updated] (FLUME-962) Failover capability for Client SDK
Posted by "Hari Shreedharan (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hari Shreedharan updated FLUME-962:
-----------------------------------
Attachment: FLUME-962-rebased-1.patch
Rebased on trunk
> 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
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13233571#comment-13233571 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-20 17:14:35.255534)
Review request for Flume.
Changes
-------
Removed the other getFailoverInstance method from RpcClientFactory.
Summary
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 9497a3d
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java 8c40aa4
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java 0c94231
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
> 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
>
>
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231685#comment-13231685 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-16 22:22:27.859970)
Review request for Flume.
Summary
-------
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs
-----
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 9497a3d
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
Diff: https://reviews.apache.org/r/4380/diff
Testing (updated)
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
> Fix For: v1.2.0
>
> Attachments: FLUME-962-2.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240879#comment-13240879 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6502
-----------------------------------------------------------
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java
<https://reviews.apache.org/r/4380/#comment14160>
Just a minor thing, let's fix the indentation here.
- Mike
On 2012-03-28 07:25:01, 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 07:25:01)
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/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/NettyAvroRpcClient.java 965b2ff
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, FLUME-962-rebased-4.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13237797#comment-13237797 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-25 08:06:34.877949)
Review request for Flume.
Changes
-------
Incorporated Mike's feedback. I feel it is not necessary to namespace and make the configuration properties too complex. We should not be making it too complex by making the syntax unnecessarily complex. Also I have not changed the reflection code logic in the factory, but made it more exception-safe. I cannot think of a way of writing a reflection-free, by still keeping it extensible(allowing more types of clients to be added). This is the standard technique used in most of the Flume factory classes.
Summary
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java a33e9c8
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java 0c94231
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 93bfee9
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240881#comment-13240881 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
bq. On 2012-03-29 01:09:32, Arvind Prabhakar wrote:
bq. > flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java, lines 93-94
bq. > <https://reviews.apache.org/r/4380/diff/22/?file=97201#file97201line93>
bq. >
bq. > Rename to getDefaultClientInstance(...)
This will require many other classes which call this to change. Should I go ahead and do that? Same for the other getInstance function.
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/#review6499
-----------------------------------------------------------
On 2012-03-28 07:25:01, 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 07:25:01)
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/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/NettyAvroRpcClient.java 965b2ff
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, FLUME-962-rebased-4.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13240186#comment-13240186 ]
jiraposter@reviews.apache.org commented on FLUME-962:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4380/
-----------------------------------------------------------
(Updated 2012-03-28 05:22:34.962111)
Review request for Flume.
Summary
-------
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.
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.
This addresses bug FLUME-962.
https://issues.apache.org/jira/browse/FLUME-962
Diffs (updated)
-----
flume-ng-sdk/src/main/java/org/apache/flume/api/AbstractRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 965b2ff
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClient.java a601213
flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 351b5b1
flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 93bfee9
flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java PRE-CREATION
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java a33e9c8
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java 0c94231
Diff: https://reviews.apache.org/r/4380/diff
Testing
-------
Unit tests added for the new functionality
Thanks,
Hari
> 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, FLUME-962-rebased-4.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
[jira] [Commented] (FLUME-962) Failover capability for Client SDK
Posted by "Hari Shreedharan (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13230362#comment-13230362 ]
Hari Shreedharan commented on FLUME-962:
----------------------------------------
Thanks Arvind, that clears up some doubts. But do we really want to take a properties or configuration file? This will be provided as an API right? So I think we should just take a map or list of end points, rather than having to force a configuration file on the client. My idea was to provide the basic connect/send/disconnect API, and keep track of the end points internally, for this I don't see why a conf or properties would be required. Maybe the client code generates this list on the fly(say they are running on EC2 or some shared cluster?), in which case a config file or properties object gives us no real advantage.
> 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
> Fix For: v1.2.0
>
>
> 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
[jira] [Updated] (FLUME-962) Failover capability for Client SDK
Posted by "Hari Shreedharan (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FLUME-962?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hari Shreedharan updated FLUME-962:
-----------------------------------
Attachment: FLUME-962-2.patch
> 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
> Fix For: v1.2.0
>
> Attachments: FLUME-962-2.patch, FLUME-962-2.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