Re: [PR] CAMEL-20794: AWS2 Kinesis producer supports sending batch [camel]
fanyang commented on PR #14618: URL: https://github.com/apache/camel/pull/14618#issuecomment-2186040278 Sure, will do. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] CAMEL-20794: AWS2 Kinesis producer supports sending batch [camel]
davsclaus commented on PR #14618: URL: https://github.com/apache/camel/pull/14618#issuecomment-2186015741 It would be good to update the docs as well, do you mind creating another PR to update src/main/docs (see this folder) and add some information about sending a batch if the payload is iterarable such as a List -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] CAMEL-20794: AWS2 Kinesis producer supports sending batch [camel]
davsclaus merged PR #14618: URL: https://github.com/apache/camel/pull/14618 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] CAMEL-20794: AWS2 Kinesis producer supports sending batch [camel]
github-actions[bot] commented on PR #14618: URL: https://github.com/apache/camel/pull/14618#issuecomment-2185790135 :robot: The Apache Camel test robot will run the tests for you :+1: -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] CAMEL-20794: AWS2 Kinesis producer supports sending batch [camel]
davsclaus commented on PR #14618: URL: https://github.com/apache/camel/pull/14618#issuecomment-2185789395 /component-test camel-aws -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] CAMEL-20794: AWS2 Kinesis producer supports sending batch [camel]
davsclaus commented on PR #14606: URL: https://github.com/apache/camel/pull/14606#issuecomment-2180611770 Would you be able to add a new option on the endpoint (and component) to turn on this functionality. As this kind of batching only works for camel batching component that has the BATCH_COMPLETE flag. So it will only be applicate under some situations. And in case you shutdown Camel then if there are any pending messages in-memory then the batch should be auto complete and send during this process to avoid loosing those messages. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] CAMEL-20794: AWS2 Kinesis producer supports sending batch [camel]
davsclaus commented on PR #14241: URL: https://github.com/apache/camel/pull/14241#issuecomment-2164397267 This will not work with concurrent batch from a to azure from b to azure the array list should likely be a concurrent list, and when you flush then you would need some kind of synchronization to avoid a race condition where the list is clear and a message is added at the same time, could lead to lost message. Also this batching is okay for mixing concurrent messages in the same batch as they are also individual records being send together to azure - to speedup this. There is no upper size on azure, what if the batch has 50.000 elemenents ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] CAMEL-20794: AWS2 Kinesis producer supports sending batch [camel]
fanyang commented on PR #14241: URL: https://github.com/apache/camel/pull/14241#issuecomment-2144908086 Hi @davsclaus @oscerd Thank you for the comments. The purpose of this design is to ensure that the batch sent by the upstream sender is consistent with the batch sent by the camel kinesis producer. For example: Upstream sender sent batch 0: record 0 to 4 camel kinesis producer sent batch 0: record 0 to 4 Upstream sender marked record 0 to 4 were sent Upstream sender sent batch 1: record 5 to 9 camel kinesis producer sent batch 1: record 5 to 9 Upstream sender marked record 5 to 9 were sent Once camel kinesis producer failed to send batch 1, then upstream sender will resend record 5 to 9. If the batch boundary is inconsistent between camel kinesis producer and upstream send, the above commit mechanism will be invalid, and data loss may occur. So, we cannot let the camel kinesis producer control the boundaries of the batch to avoid inconsistent batches. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] CAMEL-20794: AWS2 Kinesis producer supports sending batch [camel]
davsclaus commented on PR #14241: URL: https://github.com/apache/camel/pull/14241#issuecomment-2139847495 Yes there is also multi threading and concurrency to consider, and batching per group, and have a way of a batch to timeout and whatnot. Its usually more complex than at first glance. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] CAMEL-20794: AWS2 Kinesis producer supports sending batch [camel]
oscerd commented on code in PR #14241:
URL: https://github.com/apache/camel/pull/14241#discussion_r1620483018
##
components/camel-aws/camel-aws2-kinesis/src/main/java/org/apache/camel/component/aws2/kinesis/Kinesis2Producer.java:
##
@@ -69,8 +96,37 @@ private PutRecordRequest createRequest(Exchange exchange) {
return putRecordRequest.build();
}
-public static Message getMessageForResponse(final Exchange exchange) {
-return exchange.getMessage();
+private PutRecordsRequestEntry createRequestEntry(Exchange exchange) {
Review Comment:
There is no security about the presence of the header.
##
components/camel-aws/camel-aws2-kinesis/src/main/java/org/apache/camel/component/aws2/kinesis/Kinesis2Producer.java:
##
@@ -47,9 +59,24 @@ public Kinesis2Endpoint getEndpoint() {
@Override
public void process(Exchange exchange) throws Exception {
+Boolean batchComplete = exchange.getProperty(Exchange.BATCH_COMPLETE,
Boolean.class);
+if (batchComplete == null) {
+flushRequestBatch();
+sendSingleRecord(exchange);
+return;
+}
+
+this.requestBatch.add(createRequestEntry(exchange));
+if (batchComplete) {
+flushRequestBatch();
+}
+}
+
+private void sendSingleRecord(Exchange exchange) {
PutRecordRequest request = createRequest(exchange);
PutRecordResponse putRecordResult =
connection.getClient(getEndpoint()).putRecord(request);
-Message message = getMessageForResponse(exchange);
+LOG.trace("Sent 1 record.");
Review Comment:
I think this is useless without details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] CAMEL-20794: AWS2 Kinesis producer supports sending batch [camel]
github-actions[bot] commented on PR #14241: URL: https://github.com/apache/camel/pull/14241#issuecomment-2132189215 :star2: Thank you for your contribution to the Apache Camel project! :star2: :robot: CI automation will test this PR automatically. :camel: Apache Camel Committers, please review the following items: * First-time contributors **require MANUAL approval** for the GitHub Actions to run * You can use the command `/component-test (camel-)component-name1 (camel-)component-name2..` to request a test from the test bot. * You can label PRs using `build-all`, `build-dependents`, `skip-tests` and `test-dependents` to fine-tune the checks executed by this PR. * Build and test logs are available in the Summary page. **Only** [Apache Camel committers](https://camel.apache.org/community/team/#committers) have access to the summary. * :warning: Be careful when sharing logs. Review their contents before sharing them publicly. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] CAMEL-20794: AWS2 Kinesis producer supports sending batch [camel]
fanyang closed pull request #14238: CAMEL-20794: AWS2 Kinesis producer supports sending batch URL: https://github.com/apache/camel/pull/14238 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] CAMEL-20794: AWS2 Kinesis producer supports sending batch [camel]
fanyang commented on code in PR #14238:
URL: https://github.com/apache/camel/pull/14238#discussion_r1614645768
##
components/camel-aws/camel-aws2-kinesis/src/main/java/org/apache/camel/component/aws2/kinesis/Kinesis2Producer.java:
##
@@ -47,11 +53,28 @@ public Kinesis2Endpoint getEndpoint() {
@Override
public void process(Exchange exchange) throws Exception {
+Exception exceptionThrownByPutRecord = this.exceptionThrownByPutRecord;
Review Comment:
Hi Claus, thanks for the comments. This PR keeps using current client and
supports sending batch.
Since sending batch already has a good throughput, and as you said async
process is complex, It seems that we don't really need an async client.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] CAMEL-20794: AWS2 Kinesis producer supports sending batch [camel]
fanyang commented on code in PR #14238:
URL: https://github.com/apache/camel/pull/14238#discussion_r1614642796
##
components/camel-aws/camel-aws2-kinesis/src/main/java/org/apache/camel/component/aws2/kinesis/Kinesis2Producer.java:
##
@@ -47,11 +53,28 @@ public Kinesis2Endpoint getEndpoint() {
@Override
public void process(Exchange exchange) throws Exception {
+Exception exceptionThrownByPutRecord = this.exceptionThrownByPutRecord;
+if (exceptionThrownByPutRecord != null) {
+this.exceptionThrownByPutRecord = null;
+throw exceptionThrownByPutRecord;
+}
+
PutRecordRequest request = createRequest(exchange);
-PutRecordResponse putRecordResult =
connection.getClient(getEndpoint()).putRecord(request);
-Message message = getMessageForResponse(exchange);
-message.setHeader(Kinesis2Constants.SEQUENCE_NUMBER,
putRecordResult.sequenceNumber());
-message.setHeader(Kinesis2Constants.SHARD_ID,
putRecordResult.shardId());
+if (getEndpoint().getConfiguration().isAsyncClient()) {
Review Comment:
Thanks for the comments. Keep using synchronous client.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
