You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "Roiocam (via GitHub)" <gi...@apache.org> on 2024/02/08 15:24:48 UTC
[PR] fix Kafka flaky test [incubator-pekko-projection]
Roiocam opened a new pull request, #105:
URL: https://github.com/apache/incubator-pekko-projection/pull/105
Trying to solve #103, I am very doubtful that KafakConsumer will not be consumed evenly from two partitions.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix Kafka flaky test [incubator-pekko-projection]
Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on code in PR #105:
URL: https://github.com/apache/incubator-pekko-projection/pull/105#discussion_r1495290031
##########
kafka-test/src/test/scala/org/apache/pekko/projection/kafka/internal/KafkaSourceProviderImplSpec.scala:
##########
@@ -109,6 +110,12 @@ class KafkaSourceProviderImplSpec extends ScalaTestWithActorTestKit with LogCapt
records.count(_.partition() == tp1.partition()) shouldBe 5
}
+ // because source push to handle(probe) before sinkProbe request pull, it made probe cache random one record
+ val eagerMessage = probe.receiveMessage()
Review Comment:
It's kind of like https://github.com/akka/akka-projection/issues/462, i won't say this is best solution.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix Kafka flaky test [incubator-pekko-projection]
Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on code in PR #105:
URL: https://github.com/apache/incubator-pekko-projection/pull/105#discussion_r1495297199
##########
kafka-test/src/test/scala/org/apache/pekko/projection/kafka/internal/KafkaSourceProviderImplSpec.scala:
##########
@@ -109,6 +110,12 @@ class KafkaSourceProviderImplSpec extends ScalaTestWithActorTestKit with LogCapt
records.count(_.partition() == tp1.partition()) shouldBe 5
}
+ // because source push to handle(probe) before sinkProbe request pull, it made probe cache random one record
+ val eagerMessage = probe.receiveMessage()
Review Comment:
@He-Pin Is there any way to make the source and sink reach a consensus on the number of elements?
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix Kafka flaky test [incubator-pekko-projection]
Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on PR #105:
URL: https://github.com/apache/incubator-pekko-projection/pull/105#issuecomment-1934409391
After four times CI checks(see: https://github.com/apache/incubator-pekko-projection/actions/runs/7831923269), this kind of test has been successfully passed, and we should be able to assume that this kind of flaky has been fixed.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix Kafka flaky test [incubator-pekko-projection]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on PR #105:
URL: https://github.com/apache/incubator-pekko-projection/pull/105#issuecomment-1935811477
> > TBH I'm not convinced this change fixes the root cause of the instability: because of the assertion `records.count(_.partition() == tp0.partition()) shouldBe 5` AFAICS it should be guaranteed that `tp0TestCount` would always be 5, so this change should not have an effect other than timing differences - or did I miss something?
>
> The producer produces a total of 20 elements, and each partition has 10 evenly.
>
> Then Sink consumes 10 elements from two partitions. There is no assertion that each partition consumes 5 evenly.
Isn't that asserted by https://github.com/apache/incubator-pekko-projection/pull/105/files#diff-fdbaabc373547fac2368cc881beaa945f8d0225553d3e964dbe09eaf792592adR108-R112 ?
> If it is not consumed evenly, it will lead to problems with subsequent assertions, because partition 0 may consume 6 records in the previous step.
>
> This PR does not force the assertion that Sink consumes evenly from all partitions. If Kafka Consumer is not balanced consume from partitions, I don't think this is the responsibility of this test or should be added a new test to assert.
That makes sense to me.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix Kafka flaky test [incubator-pekko-projection]
Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on PR #105:
URL: https://github.com/apache/incubator-pekko-projection/pull/105#issuecomment-1934414814
@raboof Could you please do a code review? 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 above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix Kafka flaky test [incubator-pekko-projection]
Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on PR #105:
URL: https://github.com/apache/incubator-pekko-projection/pull/105#issuecomment-1935793124
> TBH I'm not convinced this change fixes the root cause of the instability: because of the assertion `records.count(_.partition() == tp0.partition()) shouldBe 5` AFAICS it should be guaranteed that `tp0TestCount` would always be 5, so this change should not have an effect other than timing differences - or did I miss something?
The producer produces a total of 20 elements, and each partition has 10 evenly.
Then Sink consumes 10 elements from two partitions. There is no assertion that each partition consumes 5 evenly. If it is not consumed evenly, it will lead to problems with subsequent assertions, because partition 0 may consume 6 records in the previous step.
This PR does not force the assertion that Sink consumes evenly from all partitions. If Kafka Consumer is not balanced consume from partitions, I don't think this is the responsibility of this test or should be added a new test to assert.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix Kafka flaky test [pekko-projection]
Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on PR #105:
URL: https://github.com/apache/pekko-projection/pull/105#issuecomment-2042261946
Would you like to take a look? @raboof
This may not the best solution, but it does reduce flaky.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix Kafka flaky test [pekko-projection]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof merged PR #105:
URL: https://github.com/apache/pekko-projection/pull/105
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org