[jira] [Commented] (KAFKA-2123) Make new consumer offset commit API use callback + future
[ https://issues.apache.org/jira/browse/KAFKA-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14629067#comment-14629067 ] ASF GitHub Bot commented on KAFKA-2123: --- Github user asfgit closed the pull request at: https://github.com/apache/kafka/pull/79 > Make new consumer offset commit API use callback + future > - > > Key: KAFKA-2123 > URL: https://issues.apache.org/jira/browse/KAFKA-2123 > Project: Kafka > Issue Type: Sub-task > Components: clients, consumer >Reporter: Ewen Cheslack-Postava >Assignee: Jason Gustafson >Priority: Critical > Fix For: 0.8.3 > > Attachments: KAFKA-2123.patch, KAFKA-2123.patch, > KAFKA-2123_2015-04-30_11:23:05.patch, KAFKA-2123_2015-05-01_19:33:19.patch, > KAFKA-2123_2015-05-04_09:39:50.patch, KAFKA-2123_2015-05-04_22:51:48.patch, > KAFKA-2123_2015-05-29_11:11:05.patch, KAFKA-2123_2015-07-11_17:33:59.patch, > KAFKA-2123_2015-07-13_18:45:08.patch, KAFKA-2123_2015-07-14_13:20:25.patch, > KAFKA-2123_2015-07-14_18:21:38.patch > > > The current version of the offset commit API in the new consumer is > void commit(offsets, commit type) > where the commit type is either sync or async. This means you need to use > sync if you ever want confirmation that the commit succeeded. Some > applications will want to use asynchronous offset commit, but be able to tell > when the commit completes. > This is basically the same problem that had to be fixed going from old > consumer -> new consumer and I'd suggest the same fix using a callback + > future combination. The new API would be > Future commit(Map offsets, ConsumerCommitCallback > callback); > where ConsumerCommitCallback contains a single method: > public void onCompletion(Exception exception); > We can provide shorthand variants of commit() for eliding the different > arguments. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (KAFKA-2123) Make new consumer offset commit API use callback + future
[ https://issues.apache.org/jira/browse/KAFKA-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14628994#comment-14628994 ] ASF GitHub Bot commented on KAFKA-2123: --- GitHub user hachikuji opened a pull request: https://github.com/apache/kafka/pull/79 [Minor] fix new consumer heartbeat reschedule bug This commit fixes a minor issue introduced in the patch for KAFKA-2123. The schedule method requires the time the task should be executed, not a delay. You can merge this pull request into a Git repository by running: $ git pull https://github.com/hachikuji/kafka KAFKA-2123-fix Alternatively you can review and apply these changes as the patch at: https://github.com/apache/kafka/pull/79.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #79 commit 6eb7ec648fdd95e9c73cf6c452c425527e6c800d Author: Jason Gustafson Date: 2015-07-16T00:10:12Z [Minor] fix new consumer heartbeat reschedule bug > Make new consumer offset commit API use callback + future > - > > Key: KAFKA-2123 > URL: https://issues.apache.org/jira/browse/KAFKA-2123 > Project: Kafka > Issue Type: Sub-task > Components: clients, consumer >Reporter: Ewen Cheslack-Postava >Assignee: Jason Gustafson >Priority: Critical > Fix For: 0.8.3 > > Attachments: KAFKA-2123.patch, KAFKA-2123.patch, > KAFKA-2123_2015-04-30_11:23:05.patch, KAFKA-2123_2015-05-01_19:33:19.patch, > KAFKA-2123_2015-05-04_09:39:50.patch, KAFKA-2123_2015-05-04_22:51:48.patch, > KAFKA-2123_2015-05-29_11:11:05.patch, KAFKA-2123_2015-07-11_17:33:59.patch, > KAFKA-2123_2015-07-13_18:45:08.patch, KAFKA-2123_2015-07-14_13:20:25.patch, > KAFKA-2123_2015-07-14_18:21:38.patch > > > The current version of the offset commit API in the new consumer is > void commit(offsets, commit type) > where the commit type is either sync or async. This means you need to use > sync if you ever want confirmation that the commit succeeded. Some > applications will want to use asynchronous offset commit, but be able to tell > when the commit completes. > This is basically the same problem that had to be fixed going from old > consumer -> new consumer and I'd suggest the same fix using a callback + > future combination. The new API would be > Future commit(Map offsets, ConsumerCommitCallback > callback); > where ConsumerCommitCallback contains a single method: > public void onCompletion(Exception exception); > We can provide shorthand variants of commit() for eliding the different > arguments. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (KAFKA-2123) Make new consumer offset commit API use callback + future
[ https://issues.apache.org/jira/browse/KAFKA-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14628606#comment-14628606 ] Guozhang Wang commented on KAFKA-2123: -- Thanks [~hachikuji] for the patience working on this long-dragging patch! This is great work and I have just committed to trunk. > Make new consumer offset commit API use callback + future > - > > Key: KAFKA-2123 > URL: https://issues.apache.org/jira/browse/KAFKA-2123 > Project: Kafka > Issue Type: Sub-task > Components: clients, consumer >Reporter: Ewen Cheslack-Postava >Assignee: Ewen Cheslack-Postava >Priority: Critical > Fix For: 0.8.3 > > Attachments: KAFKA-2123.patch, KAFKA-2123.patch, > KAFKA-2123_2015-04-30_11:23:05.patch, KAFKA-2123_2015-05-01_19:33:19.patch, > KAFKA-2123_2015-05-04_09:39:50.patch, KAFKA-2123_2015-05-04_22:51:48.patch, > KAFKA-2123_2015-05-29_11:11:05.patch, KAFKA-2123_2015-07-11_17:33:59.patch, > KAFKA-2123_2015-07-13_18:45:08.patch, KAFKA-2123_2015-07-14_13:20:25.patch, > KAFKA-2123_2015-07-14_18:21:38.patch > > > The current version of the offset commit API in the new consumer is > void commit(offsets, commit type) > where the commit type is either sync or async. This means you need to use > sync if you ever want confirmation that the commit succeeded. Some > applications will want to use asynchronous offset commit, but be able to tell > when the commit completes. > This is basically the same problem that had to be fixed going from old > consumer -> new consumer and I'd suggest the same fix using a callback + > future combination. The new API would be > Future commit(Map offsets, ConsumerCommitCallback > callback); > where ConsumerCommitCallback contains a single method: > public void onCompletion(Exception exception); > We can provide shorthand variants of commit() for eliding the different > arguments. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (KAFKA-2123) Make new consumer offset commit API use callback + future
[ https://issues.apache.org/jira/browse/KAFKA-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14627383#comment-14627383 ] Jason Gustafson commented on KAFKA-2123: Updated reviewboard https://reviews.apache.org/r/36333/diff/ against branch upstream/trunk > Make new consumer offset commit API use callback + future > - > > Key: KAFKA-2123 > URL: https://issues.apache.org/jira/browse/KAFKA-2123 > Project: Kafka > Issue Type: Sub-task > Components: clients, consumer >Reporter: Ewen Cheslack-Postava >Assignee: Ewen Cheslack-Postava >Priority: Critical > Fix For: 0.8.3 > > Attachments: KAFKA-2123.patch, KAFKA-2123.patch, > KAFKA-2123_2015-04-30_11:23:05.patch, KAFKA-2123_2015-05-01_19:33:19.patch, > KAFKA-2123_2015-05-04_09:39:50.patch, KAFKA-2123_2015-05-04_22:51:48.patch, > KAFKA-2123_2015-05-29_11:11:05.patch, KAFKA-2123_2015-07-11_17:33:59.patch, > KAFKA-2123_2015-07-13_18:45:08.patch, KAFKA-2123_2015-07-14_13:20:25.patch, > KAFKA-2123_2015-07-14_18:21:38.patch > > > The current version of the offset commit API in the new consumer is > void commit(offsets, commit type) > where the commit type is either sync or async. This means you need to use > sync if you ever want confirmation that the commit succeeded. Some > applications will want to use asynchronous offset commit, but be able to tell > when the commit completes. > This is basically the same problem that had to be fixed going from old > consumer -> new consumer and I'd suggest the same fix using a callback + > future combination. The new API would be > Future commit(Map offsets, ConsumerCommitCallback > callback); > where ConsumerCommitCallback contains a single method: > public void onCompletion(Exception exception); > We can provide shorthand variants of commit() for eliding the different > arguments. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (KAFKA-2123) Make new consumer offset commit API use callback + future
[ https://issues.apache.org/jira/browse/KAFKA-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14626987#comment-14626987 ] Jason Gustafson commented on KAFKA-2123: Updated reviewboard https://reviews.apache.org/r/36333/diff/ against branch upstream/trunk > Make new consumer offset commit API use callback + future > - > > Key: KAFKA-2123 > URL: https://issues.apache.org/jira/browse/KAFKA-2123 > Project: Kafka > Issue Type: Sub-task > Components: clients, consumer >Reporter: Ewen Cheslack-Postava >Assignee: Ewen Cheslack-Postava >Priority: Critical > Fix For: 0.8.3 > > Attachments: KAFKA-2123.patch, KAFKA-2123.patch, > KAFKA-2123_2015-04-30_11:23:05.patch, KAFKA-2123_2015-05-01_19:33:19.patch, > KAFKA-2123_2015-05-04_09:39:50.patch, KAFKA-2123_2015-05-04_22:51:48.patch, > KAFKA-2123_2015-05-29_11:11:05.patch, KAFKA-2123_2015-07-11_17:33:59.patch, > KAFKA-2123_2015-07-13_18:45:08.patch, KAFKA-2123_2015-07-14_13:20:25.patch > > > The current version of the offset commit API in the new consumer is > void commit(offsets, commit type) > where the commit type is either sync or async. This means you need to use > sync if you ever want confirmation that the commit succeeded. Some > applications will want to use asynchronous offset commit, but be able to tell > when the commit completes. > This is basically the same problem that had to be fixed going from old > consumer -> new consumer and I'd suggest the same fix using a callback + > future combination. The new API would be > Future commit(Map offsets, ConsumerCommitCallback > callback); > where ConsumerCommitCallback contains a single method: > public void onCompletion(Exception exception); > We can provide shorthand variants of commit() for eliding the different > arguments. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (KAFKA-2123) Make new consumer offset commit API use callback + future
[ https://issues.apache.org/jira/browse/KAFKA-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14625677#comment-14625677 ] Jason Gustafson commented on KAFKA-2123: Updated reviewboard https://reviews.apache.org/r/36333/diff/ against branch upstream/trunk > Make new consumer offset commit API use callback + future > - > > Key: KAFKA-2123 > URL: https://issues.apache.org/jira/browse/KAFKA-2123 > Project: Kafka > Issue Type: Sub-task > Components: clients, consumer >Reporter: Ewen Cheslack-Postava >Assignee: Ewen Cheslack-Postava >Priority: Critical > Fix For: 0.8.3 > > Attachments: KAFKA-2123.patch, KAFKA-2123.patch, > KAFKA-2123_2015-04-30_11:23:05.patch, KAFKA-2123_2015-05-01_19:33:19.patch, > KAFKA-2123_2015-05-04_09:39:50.patch, KAFKA-2123_2015-05-04_22:51:48.patch, > KAFKA-2123_2015-05-29_11:11:05.patch, KAFKA-2123_2015-07-11_17:33:59.patch, > KAFKA-2123_2015-07-13_18:45:08.patch > > > The current version of the offset commit API in the new consumer is > void commit(offsets, commit type) > where the commit type is either sync or async. This means you need to use > sync if you ever want confirmation that the commit succeeded. Some > applications will want to use asynchronous offset commit, but be able to tell > when the commit completes. > This is basically the same problem that had to be fixed going from old > consumer -> new consumer and I'd suggest the same fix using a callback + > future combination. The new API would be > Future commit(Map offsets, ConsumerCommitCallback > callback); > where ConsumerCommitCallback contains a single method: > public void onCompletion(Exception exception); > We can provide shorthand variants of commit() for eliding the different > arguments. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (KAFKA-2123) Make new consumer offset commit API use callback + future
[ https://issues.apache.org/jira/browse/KAFKA-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14623637#comment-14623637 ] Jason Gustafson commented on KAFKA-2123: Updated reviewboard https://reviews.apache.org/r/36333/diff/ against branch upstream/trunk > Make new consumer offset commit API use callback + future > - > > Key: KAFKA-2123 > URL: https://issues.apache.org/jira/browse/KAFKA-2123 > Project: Kafka > Issue Type: Sub-task > Components: clients, consumer >Reporter: Ewen Cheslack-Postava >Assignee: Ewen Cheslack-Postava >Priority: Critical > Fix For: 0.8.3 > > Attachments: KAFKA-2123.patch, KAFKA-2123.patch, > KAFKA-2123_2015-04-30_11:23:05.patch, KAFKA-2123_2015-05-01_19:33:19.patch, > KAFKA-2123_2015-05-04_09:39:50.patch, KAFKA-2123_2015-05-04_22:51:48.patch, > KAFKA-2123_2015-05-29_11:11:05.patch, KAFKA-2123_2015-07-11_17:33:59.patch > > > The current version of the offset commit API in the new consumer is > void commit(offsets, commit type) > where the commit type is either sync or async. This means you need to use > sync if you ever want confirmation that the commit succeeded. Some > applications will want to use asynchronous offset commit, but be able to tell > when the commit completes. > This is basically the same problem that had to be fixed going from old > consumer -> new consumer and I'd suggest the same fix using a callback + > future combination. The new API would be > Future commit(Map offsets, ConsumerCommitCallback > callback); > where ConsumerCommitCallback contains a single method: > public void onCompletion(Exception exception); > We can provide shorthand variants of commit() for eliding the different > arguments. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (KAFKA-2123) Make new consumer offset commit API use callback + future
[ https://issues.apache.org/jira/browse/KAFKA-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14619432#comment-14619432 ] Jason Gustafson commented on KAFKA-2123: [~ewencp], [~guozhang], apologies for the big commit, but I saw an opportunity to consolidate some of the features from Ewen's previous patch with those introduced in KAFKA-2168 into a general ConsumerNetworkClient. This allowed me to push the retry logic that spilled into KafkaConsumer back into Coordinator and Fetcher while still ensuring that consumer.wakeup() would continue to work and scheduled tasks (such as auto-commits and heartbeats) would get executed at the right time regardless of where the thread of execution was. I have not, however, preserved the commit queue from Ewen's initial patches, but we can bring it back if we think it adds a lot of value. > Make new consumer offset commit API use callback + future > - > > Key: KAFKA-2123 > URL: https://issues.apache.org/jira/browse/KAFKA-2123 > Project: Kafka > Issue Type: Sub-task > Components: clients, consumer >Reporter: Ewen Cheslack-Postava >Assignee: Ewen Cheslack-Postava >Priority: Critical > Fix For: 0.8.3 > > Attachments: KAFKA-2123.patch, KAFKA-2123.patch, > KAFKA-2123_2015-04-30_11:23:05.patch, KAFKA-2123_2015-05-01_19:33:19.patch, > KAFKA-2123_2015-05-04_09:39:50.patch, KAFKA-2123_2015-05-04_22:51:48.patch, > KAFKA-2123_2015-05-29_11:11:05.patch > > > The current version of the offset commit API in the new consumer is > void commit(offsets, commit type) > where the commit type is either sync or async. This means you need to use > sync if you ever want confirmation that the commit succeeded. Some > applications will want to use asynchronous offset commit, but be able to tell > when the commit completes. > This is basically the same problem that had to be fixed going from old > consumer -> new consumer and I'd suggest the same fix using a callback + > future combination. The new API would be > Future commit(Map offsets, ConsumerCommitCallback > callback); > where ConsumerCommitCallback contains a single method: > public void onCompletion(Exception exception); > We can provide shorthand variants of commit() for eliding the different > arguments. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (KAFKA-2123) Make new consumer offset commit API use callback + future
[ https://issues.apache.org/jira/browse/KAFKA-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14619386#comment-14619386 ] Jason Gustafson commented on KAFKA-2123: Created reviewboard https://reviews.apache.org/r/36333/diff/ against branch upstream/trunk > Make new consumer offset commit API use callback + future > - > > Key: KAFKA-2123 > URL: https://issues.apache.org/jira/browse/KAFKA-2123 > Project: Kafka > Issue Type: Sub-task > Components: clients, consumer >Reporter: Ewen Cheslack-Postava >Assignee: Ewen Cheslack-Postava >Priority: Critical > Fix For: 0.8.3 > > Attachments: KAFKA-2123.patch, KAFKA-2123.patch, > KAFKA-2123_2015-04-30_11:23:05.patch, KAFKA-2123_2015-05-01_19:33:19.patch, > KAFKA-2123_2015-05-04_09:39:50.patch, KAFKA-2123_2015-05-04_22:51:48.patch, > KAFKA-2123_2015-05-29_11:11:05.patch > > > The current version of the offset commit API in the new consumer is > void commit(offsets, commit type) > where the commit type is either sync or async. This means you need to use > sync if you ever want confirmation that the commit succeeded. Some > applications will want to use asynchronous offset commit, but be able to tell > when the commit completes. > This is basically the same problem that had to be fixed going from old > consumer -> new consumer and I'd suggest the same fix using a callback + > future combination. The new API would be > Future commit(Map offsets, ConsumerCommitCallback > callback); > where ConsumerCommitCallback contains a single method: > public void onCompletion(Exception exception); > We can provide shorthand variants of commit() for eliding the different > arguments. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (KAFKA-2123) Make new consumer offset commit API use callback + future
[ https://issues.apache.org/jira/browse/KAFKA-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14565164#comment-14565164 ] Ewen Cheslack-Postava commented on KAFKA-2123: -- Updated reviewboard https://reviews.apache.org/r/33196/diff/ against branch origin/trunk > Make new consumer offset commit API use callback + future > - > > Key: KAFKA-2123 > URL: https://issues.apache.org/jira/browse/KAFKA-2123 > Project: Kafka > Issue Type: Improvement > Components: clients, consumer >Reporter: Ewen Cheslack-Postava >Assignee: Ewen Cheslack-Postava > Fix For: 0.8.3 > > Attachments: KAFKA-2123.patch, KAFKA-2123_2015-04-30_11:23:05.patch, > KAFKA-2123_2015-05-01_19:33:19.patch, KAFKA-2123_2015-05-04_09:39:50.patch, > KAFKA-2123_2015-05-04_22:51:48.patch, KAFKA-2123_2015-05-29_11:11:05.patch > > > The current version of the offset commit API in the new consumer is > void commit(offsets, commit type) > where the commit type is either sync or async. This means you need to use > sync if you ever want confirmation that the commit succeeded. Some > applications will want to use asynchronous offset commit, but be able to tell > when the commit completes. > This is basically the same problem that had to be fixed going from old > consumer -> new consumer and I'd suggest the same fix using a callback + > future combination. The new API would be > Future commit(Map offsets, ConsumerCommitCallback > callback); > where ConsumerCommitCallback contains a single method: > public void onCompletion(Exception exception); > We can provide shorthand variants of commit() for eliding the different > arguments. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (KAFKA-2123) Make new consumer offset commit API use callback + future
[ https://issues.apache.org/jira/browse/KAFKA-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14527975#comment-14527975 ] Ewen Cheslack-Postava commented on KAFKA-2123: -- Updated reviewboard https://reviews.apache.org/r/33196/diff/ against branch origin/trunk > Make new consumer offset commit API use callback + future > - > > Key: KAFKA-2123 > URL: https://issues.apache.org/jira/browse/KAFKA-2123 > Project: Kafka > Issue Type: Improvement > Components: clients, consumer >Reporter: Ewen Cheslack-Postava >Assignee: Ewen Cheslack-Postava > Fix For: 0.8.3 > > Attachments: KAFKA-2123.patch, KAFKA-2123_2015-04-30_11:23:05.patch, > KAFKA-2123_2015-05-01_19:33:19.patch, KAFKA-2123_2015-05-04_09:39:50.patch, > KAFKA-2123_2015-05-04_22:51:48.patch > > > The current version of the offset commit API in the new consumer is > void commit(offsets, commit type) > where the commit type is either sync or async. This means you need to use > sync if you ever want confirmation that the commit succeeded. Some > applications will want to use asynchronous offset commit, but be able to tell > when the commit completes. > This is basically the same problem that had to be fixed going from old > consumer -> new consumer and I'd suggest the same fix using a callback + > future combination. The new API would be > Future commit(Map offsets, ConsumerCommitCallback > callback); > where ConsumerCommitCallback contains a single method: > public void onCompletion(Exception exception); > We can provide shorthand variants of commit() for eliding the different > arguments. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (KAFKA-2123) Make new consumer offset commit API use callback + future
[ https://issues.apache.org/jira/browse/KAFKA-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14526814#comment-14526814 ] Ewen Cheslack-Postava commented on KAFKA-2123: -- Updated reviewboard https://reviews.apache.org/r/33196/diff/ against branch origin/trunk > Make new consumer offset commit API use callback + future > - > > Key: KAFKA-2123 > URL: https://issues.apache.org/jira/browse/KAFKA-2123 > Project: Kafka > Issue Type: Improvement > Components: clients, consumer >Reporter: Ewen Cheslack-Postava >Assignee: Ewen Cheslack-Postava > Fix For: 0.8.3 > > Attachments: KAFKA-2123.patch, KAFKA-2123_2015-04-30_11:23:05.patch, > KAFKA-2123_2015-05-01_19:33:19.patch, KAFKA-2123_2015-05-04_09:39:50.patch > > > The current version of the offset commit API in the new consumer is > void commit(offsets, commit type) > where the commit type is either sync or async. This means you need to use > sync if you ever want confirmation that the commit succeeded. Some > applications will want to use asynchronous offset commit, but be able to tell > when the commit completes. > This is basically the same problem that had to be fixed going from old > consumer -> new consumer and I'd suggest the same fix using a callback + > future combination. The new API would be > Future commit(Map offsets, ConsumerCommitCallback > callback); > where ConsumerCommitCallback contains a single method: > public void onCompletion(Exception exception); > We can provide shorthand variants of commit() for eliding the different > arguments. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (KAFKA-2123) Make new consumer offset commit API use callback + future
[ https://issues.apache.org/jira/browse/KAFKA-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14524491#comment-14524491 ] Ewen Cheslack-Postava commented on KAFKA-2123: -- Updated to add backoff back in upon retries, which was removed when I unified the sync and async processing. I ended up generalizing this a bit to provide for scheduling tasks that need to be delayed for some time. With the generalization, I could also move the automatic background offset commits to this system as well. This also fixed a bug in the existing implementation where a long poll might cause automatic offset commit to not run since we only checked before the poll if a commit was needed but didn't account for the need to stop polling and commit offsets if the commit should happen sometime during the poll() period. [~jkreps] and [~junrao] might want to take a look -- the producer code has gotten complicated because we have so many timeouts to coordinate, so computing the right poll timeouts has to take into account a bunch of variables. I think a centralized scheduler might help keep this complexity in check. > Make new consumer offset commit API use callback + future > - > > Key: KAFKA-2123 > URL: https://issues.apache.org/jira/browse/KAFKA-2123 > Project: Kafka > Issue Type: Improvement > Components: clients, consumer >Reporter: Ewen Cheslack-Postava >Assignee: Ewen Cheslack-Postava > Fix For: 0.8.3 > > Attachments: KAFKA-2123.patch, KAFKA-2123_2015-04-30_11:23:05.patch, > KAFKA-2123_2015-05-01_19:33:19.patch > > > The current version of the offset commit API in the new consumer is > void commit(offsets, commit type) > where the commit type is either sync or async. This means you need to use > sync if you ever want confirmation that the commit succeeded. Some > applications will want to use asynchronous offset commit, but be able to tell > when the commit completes. > This is basically the same problem that had to be fixed going from old > consumer -> new consumer and I'd suggest the same fix using a callback + > future combination. The new API would be > Future commit(Map offsets, ConsumerCommitCallback > callback); > where ConsumerCommitCallback contains a single method: > public void onCompletion(Exception exception); > We can provide shorthand variants of commit() for eliding the different > arguments. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (KAFKA-2123) Make new consumer offset commit API use callback + future
[ https://issues.apache.org/jira/browse/KAFKA-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14524487#comment-14524487 ] Ewen Cheslack-Postava commented on KAFKA-2123: -- Updated reviewboard https://reviews.apache.org/r/33196/diff/ against branch origin/trunk > Make new consumer offset commit API use callback + future > - > > Key: KAFKA-2123 > URL: https://issues.apache.org/jira/browse/KAFKA-2123 > Project: Kafka > Issue Type: Improvement > Components: clients, consumer >Reporter: Ewen Cheslack-Postava >Assignee: Ewen Cheslack-Postava > Fix For: 0.8.3 > > Attachments: KAFKA-2123.patch, KAFKA-2123_2015-04-30_11:23:05.patch, > KAFKA-2123_2015-05-01_19:33:19.patch > > > The current version of the offset commit API in the new consumer is > void commit(offsets, commit type) > where the commit type is either sync or async. This means you need to use > sync if you ever want confirmation that the commit succeeded. Some > applications will want to use asynchronous offset commit, but be able to tell > when the commit completes. > This is basically the same problem that had to be fixed going from old > consumer -> new consumer and I'd suggest the same fix using a callback + > future combination. The new API would be > Future commit(Map offsets, ConsumerCommitCallback > callback); > where ConsumerCommitCallback contains a single method: > public void onCompletion(Exception exception); > We can provide shorthand variants of commit() for eliding the different > arguments. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (KAFKA-2123) Make new consumer offset commit API use callback + future
[ https://issues.apache.org/jira/browse/KAFKA-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14522016#comment-14522016 ] Ewen Cheslack-Postava commented on KAFKA-2123: -- This version is now ready for review. I ended up going with the simpler interface discussed on the mailing list. I ended up running into too many synchronization and proper layering of code dealing with the Future implementation because of the way the synchronization is currently handled for the consumer. I'll file a separate issue for those problems and think about if there's a path to eventually getting the interface with the Future. In the mean time, the worst case is that we end up settling on this interface, which was the other candidate discussed. Some notes: * Added a commit.retries setting. This is specific to commit offset requests which is why it has the commit. prefix instead of just being retries. Also, the default setting is currently -1, which gives infinite retries (in both sync and async modes). I think that's actually a bad idea since a default that can block indefinitely seems like it's always a bad idea to me, but we'd need to discuss the alternatives. * Added queueing of offset commit requests. I debated how best to handle this. At first I was thinking we might be able to do something smarter to combine requests, but in the face of errors (especially partial errors that are isolated to one topic partition), it gets difficult to figure out how to handle callbacks. Simple queuing seems like the right solution to me, and for the vast majority of use cases it either has no impact (you used the sync mode) or has little impact (you used async to commit all offsets automatically). Only unusual cases where you're submitting the offsets map and doing partial commits might care about smarter behavior. > Make new consumer offset commit API use callback + future > - > > Key: KAFKA-2123 > URL: https://issues.apache.org/jira/browse/KAFKA-2123 > Project: Kafka > Issue Type: Improvement > Components: clients, consumer >Reporter: Ewen Cheslack-Postava >Assignee: Ewen Cheslack-Postava > Fix For: 0.8.3 > > Attachments: KAFKA-2123.patch, KAFKA-2123_2015-04-30_11:23:05.patch > > > The current version of the offset commit API in the new consumer is > void commit(offsets, commit type) > where the commit type is either sync or async. This means you need to use > sync if you ever want confirmation that the commit succeeded. Some > applications will want to use asynchronous offset commit, but be able to tell > when the commit completes. > This is basically the same problem that had to be fixed going from old > consumer -> new consumer and I'd suggest the same fix using a callback + > future combination. The new API would be > Future commit(Map offsets, ConsumerCommitCallback > callback); > where ConsumerCommitCallback contains a single method: > public void onCompletion(Exception exception); > We can provide shorthand variants of commit() for eliding the different > arguments. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (KAFKA-2123) Make new consumer offset commit API use callback + future
[ https://issues.apache.org/jira/browse/KAFKA-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14521992#comment-14521992 ] Ewen Cheslack-Postava commented on KAFKA-2123: -- Updated reviewboard https://reviews.apache.org/r/33196/diff/ against branch origin/trunk > Make new consumer offset commit API use callback + future > - > > Key: KAFKA-2123 > URL: https://issues.apache.org/jira/browse/KAFKA-2123 > Project: Kafka > Issue Type: Improvement > Components: clients, consumer >Reporter: Ewen Cheslack-Postava >Assignee: Ewen Cheslack-Postava > Fix For: 0.8.3 > > Attachments: KAFKA-2123.patch, KAFKA-2123_2015-04-30_11:23:05.patch > > > The current version of the offset commit API in the new consumer is > void commit(offsets, commit type) > where the commit type is either sync or async. This means you need to use > sync if you ever want confirmation that the commit succeeded. Some > applications will want to use asynchronous offset commit, but be able to tell > when the commit completes. > This is basically the same problem that had to be fixed going from old > consumer -> new consumer and I'd suggest the same fix using a callback + > future combination. The new API would be > Future commit(Map offsets, ConsumerCommitCallback > callback); > where ConsumerCommitCallback contains a single method: > public void onCompletion(Exception exception); > We can provide shorthand variants of commit() for eliding the different > arguments. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (KAFKA-2123) Make new consumer offset commit API use callback + future
[ https://issues.apache.org/jira/browse/KAFKA-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14495012#comment-14495012 ] Ewen Cheslack-Postava commented on KAFKA-2123: -- Created reviewboard https://reviews.apache.org/r/33196/diff/ against branch origin/trunk > Make new consumer offset commit API use callback + future > - > > Key: KAFKA-2123 > URL: https://issues.apache.org/jira/browse/KAFKA-2123 > Project: Kafka > Issue Type: Improvement > Components: clients, consumer >Reporter: Ewen Cheslack-Postava >Assignee: Ewen Cheslack-Postava > Fix For: 0.8.3 > > Attachments: KAFKA-2123.patch > > > The current version of the offset commit API in the new consumer is > void commit(offsets, commit type) > where the commit type is either sync or async. This means you need to use > sync if you ever want confirmation that the commit succeeded. Some > applications will want to use asynchronous offset commit, but be able to tell > when the commit completes. > This is basically the same problem that had to be fixed going from old > consumer -> new consumer and I'd suggest the same fix using a callback + > future combination. The new API would be > Future commit(Map offsets, ConsumerCommitCallback > callback); > where ConsumerCommitCallback contains a single method: > public void onCompletion(Exception exception); > We can provide shorthand variants of commit() for eliding the different > arguments. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
