cadonna merged PR #15585:
URL: https://github.com/apache/kafka/pull/15585
--
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: jira-unsubscr...@kafka.apache.
lianetm commented on PR #15585:
URL: https://github.com/apache/kafka/pull/15585#issuecomment-2045768269
I created the follow-up task
https://issues.apache.org/jira/browse/KAFKA-16493 to address that.
--
This is an automated message from the Apache Git Service.
To respond to the message,
dajac commented on PR #15585:
URL: https://github.com/apache/kafka/pull/15585#issuecomment-2045520630
@cadonna Yes, we can merge it and do a follow-up.
--
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 g
cadonna commented on PR #15585:
URL: https://github.com/apache/kafka/pull/15585#issuecomment-2045503262
Could we merge this PR as it is and then fix the issue @dajac in a separate
PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to
lianetm commented on PR #15585:
URL: https://github.com/apache/kafka/pull/15585#issuecomment-2045483420
Good catch @dajac , I missed that. Checking the metadata version in the
`maybeUpdateSubscriptionMetadata` before we do the actual pattern update would
definitely save some unneeded regex
dajac commented on PR #15585:
URL: https://github.com/apache/kafka/pull/15585#issuecomment-2045420088
@lianetm Thanks. So it seems that we call it on every poll. Aren't we
concerned about the cost of matching the regex? In the legacy code, I think
that we have a mechanism to refresh it only
lianetm commented on PR #15585:
URL: https://github.com/apache/kafka/pull/15585#issuecomment-2045382837
Hey @dajac , we do have the same mechanism, calling
`updatePatternSubscription` on a regular basis, as part of the consumer poll
loop
[here](https://github.com/apache/kafka/blob/4307840f
dajac commented on PR #15585:
URL: https://github.com/apache/kafka/pull/15585#issuecomment-2045351051
Thanks for working on this one. I have one question regarding the
implementation. In the legacy consumer, we have a mechanism to refresh the
subscribed topics based on the regex. This is do
cadonna commented on PR #15585:
URL: https://github.com/apache/kafka/pull/15585#issuecomment-2045332239
I restarted the build since the Java 11 build crashed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL abo
kirktrue commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1557728693
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1750,8 +1750,8 @@ private void subscribeInternal(Pattern pattern,
Phuc-Hong-Tran commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1556146632
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1750,8 +1750,8 @@ private void subscribeInternal(Pattern pat
lianetm commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1556142547
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1750,8 +1750,8 @@ private void subscribeInternal(Pattern pattern,
kirktrue commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1556133511
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1750,8 +1750,8 @@ private void subscribeInternal(Pattern pattern,
lianetm commented on PR #15585:
URL: https://github.com/apache/kafka/pull/15585#issuecomment-2043179367
@cadonna could you take a look at this one when you have a chance? Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub
lianetm commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1556104853
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1750,8 +1753,14 @@ private void subscribeInternal(Pattern pattern,
lianetm commented on PR #15585:
URL: https://github.com/apache/kafka/pull/15585#issuecomment-2042958530
Hey @Phuc-Hong-Tran, thanks for the update, left some more comments. Almost
there! Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please
lianetm commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1555969469
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1751,16 +1753,7 @@ private void subscribeInternal(Pattern pattern,
lianetm commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1555915666
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1667,6 +1669,7 @@ private void updateLastSeenEpochIfNewer(TopicPart
lianetm commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1555909363
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerSubscriptionTest.scala:
##
@@ -39,9 +39,8 @@ class PlaintextConsumerSubscriptionTest extends
AbstractCons
Phuc-Hong-Tran commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1554474003
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1750,8 +1753,14 @@ private void subscribeInternal(Pattern pa
Phuc-Hong-Tran commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1554466415
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1750,8 +1753,14 @@ private void subscribeInternal(Pattern pa
lianetm commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1550279550
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1750,8 +1753,14 @@ private void subscribeInternal(Pattern pattern,
lianetm commented on PR #15585:
URL: https://github.com/apache/kafka/pull/15585#issuecomment-2035194359
Hey @Phuc-Hong-Tran, thanks for the updates! Left some comments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use
lianetm commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1550178486
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1750,8 +1753,14 @@ private void subscribeInternal(Pattern pattern,
lianetm commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1549895965
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##
@@ -159,6 +160,224 @@ class PlaintextConsumerTest extends BaseConsumerTest {
consumeAn
lianetm commented on PR #15585:
URL: https://github.com/apache/kafka/pull/15585#issuecomment-2032154288
Hey @Phuc-Hong-Tran , any update on this one? Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL abov
Phuc-Hong-Tran commented on PR #15585:
URL: https://github.com/apache/kafka/pull/15585#issuecomment-2021544620
Thanks for the comments @lianetm @kirktrue, I'll try to address those asap
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to
Phuc-Hong-Tran commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1540153302
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1750,8 +1753,14 @@ private void subscribeInternal(Pattern pa
Phuc-Hong-Tran commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1540156766
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1750,8 +1753,14 @@ private void subscribeInternal(Pattern pa
Phuc-Hong-Tran commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1540153302
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1750,8 +1753,14 @@ private void subscribeInternal(Pattern pa
Phuc-Hong-Tran commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1540149023
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1750,8 +1753,14 @@ private void subscribeInternal(Pattern pa
Phuc-Hong-Tran commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1540147833
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1667,6 +1667,9 @@ private void updateLastSeenEpochIfNewer(To
Phuc-Hong-Tran commented on PR #15585:
URL: https://github.com/apache/kafka/pull/15585#issuecomment-2021526537
@kirktrue, regarding
https://github.com/apache/kafka/pull/15585#discussion_r1539447102, there is a
race condition bug where the metadata is not updated but the heartbeat request
i
Phuc-Hong-Tran commented on PR #15585:
URL: https://github.com/apache/kafka/pull/15585#issuecomment-2021522732
@kirktrue, regarding comment
https://github.com/apache/kafka/pull/15585#discussion_r1539447785, I decided to
call `metadata.requestUpdateForNewTopics` since there is a quite delay
lianetm commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1539955531
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/HeartbeatRequestManager.java:
##
@@ -550,6 +550,11 @@ public ConsumerGroupHeartbeatRequestData
bui
lianetm commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1539955531
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/HeartbeatRequestManager.java:
##
@@ -550,6 +550,11 @@ public ConsumerGroupHeartbeatRequestData
bui
kirktrue commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1539435022
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/HeartbeatRequestManager.java:
##
@@ -550,6 +550,11 @@ public ConsumerGroupHeartbeatRequestData
bu
lianetm commented on PR #15585:
URL: https://github.com/apache/kafka/pull/15585#issuecomment-2018835239
Hey @Phuc-Hong-Tran , thanks a lot for the PR! I had a first pass and left
some comments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please
lianetm commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1538180339
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1667,6 +1667,9 @@ private void updateLastSeenEpochIfNewer(TopicPart
lianetm commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1538173798
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/HeartbeatRequestManager.java:
##
@@ -550,6 +550,11 @@ public ConsumerGroupHeartbeatRequestData
bui
lianetm commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1538161659
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##
@@ -433,7 +433,7 @@ class PlaintextConsumerTest extends BaseConsumerTest {
*/
// TOD
lianetm commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1538161334
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##
@@ -374,7 +374,7 @@ class PlaintextConsumerTest extends BaseConsumerTest {
*/
// TOD
lianetm commented on code in PR #15585:
URL: https://github.com/apache/kafka/pull/15585#discussion_r1538158322
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##
@@ -1750,8 +1753,14 @@ private void subscribeInternal(Pattern pattern,
Phuc-Hong-Tran opened a new pull request, #15585:
URL: https://github.com/apache/kafka/pull/15585
* Fully implemented subscription using Pattern for AsyncKafkaConsumer.
* Enabled related tests for subscription using Pattern in
PlaintextConsumerTest.
--
This is an automated message from
44 matches
Mail list logo