You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/05/27 13:27:05 UTC

[GitHub] [kafka] iakunin opened a new pull request #10775: KAFKA-12668: Making MockScheduler.schedule safe to use in concurrent code

iakunin opened a new pull request #10775:
URL: https://github.com/apache/kafka/pull/10775


   Making MockScheduler.schedule safe to use in concurrent code
   by removing `tick()` call inside MockScheduler.schedule.
   
   To reproduce a bug I wrote a unit-test MockSchedulerTest. 
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] iakunin commented on pull request #10775: KAFKA-12668: Making MockScheduler.schedule safe to use in concurrent code

Posted by GitBox <gi...@apache.org>.
iakunin commented on pull request #10775:
URL: https://github.com/apache/kafka/pull/10775#issuecomment-878083043


   @chia7712 @huxihx could you have a look at this PR, please?


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] iakunin edited a comment on pull request #10775: KAFKA-12668: Making MockScheduler.schedule safe to use in concurrent code

Posted by GitBox <gi...@apache.org>.
iakunin edited a comment on pull request #10775:
URL: https://github.com/apache/kafka/pull/10775#issuecomment-884402579


   @jsancio got it. Thanks for explaining me that. I'll be waiting patiently :)


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] iakunin edited a comment on pull request #10775: KAFKA-12668: Making MockScheduler.schedule safe to use in concurrent code

Posted by GitBox <gi...@apache.org>.
iakunin edited a comment on pull request #10775:
URL: https://github.com/apache/kafka/pull/10775#issuecomment-849675662


   @jsancio hi!
   
   Could you have a look at this PR, please?


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] iakunin edited a comment on pull request #10775: KAFKA-12668: Making MockScheduler.schedule safe to use in concurrent code

Posted by GitBox <gi...@apache.org>.
iakunin edited a comment on pull request #10775:
URL: https://github.com/apache/kafka/pull/10775#issuecomment-936453432


   @chia7712 @huxihx @jsancio hey guys! Any updates on this pull request? 
   
   Maybe you have some time to have a look and give me any feedback?


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] iakunin commented on pull request #10775: KAFKA-12668: Making MockScheduler.schedule safe to use in concurrent code

Posted by GitBox <gi...@apache.org>.
iakunin commented on pull request #10775:
URL: https://github.com/apache/kafka/pull/10775#issuecomment-860042763


   @mdedetrich, many thanks for your feedback!
   
   `MockScheduler` has no any background threads to deal with. It executes all the tasks in the main thread. 
   
   As far as we all know java/scala `syncronyzed` blocks are reentrant. This violates Log's assumption since Log calls schedule while holding a lock. That's the main reason of inconsistent behavior reported in [jira-issue](https://issues.apache.org/jira/browse/KAFKA-12668).
   
   For now I see only two possible solutions:
   1. Analyze the task passed to MockScheduler: if it contains any synchronization primitives. If it doesn't then it could be executed right away. Otherwise, its execution should be suspended until the lock is released. In my opinion, this solution is too complicated. Also I'm not sure if it possible at all.
   2. Much simpler solution I've implemented in this pull-request.
   
   Maybe you could give me a hint to a better solution. Thanks in advance!


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] iakunin commented on pull request #10775: KAFKA-12668: Making MockScheduler.schedule safe to use in concurrent code

Posted by GitBox <gi...@apache.org>.
iakunin commented on pull request #10775:
URL: https://github.com/apache/kafka/pull/10775#issuecomment-849675662


   @jsancio  could you have a look, please?


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] jsancio commented on pull request #10775: KAFKA-12668: Making MockScheduler.schedule safe to use in concurrent code

Posted by GitBox <gi...@apache.org>.
jsancio commented on pull request #10775:
URL: https://github.com/apache/kafka/pull/10775#issuecomment-884274297


   @iakunin excuse for the delays reviewing this but a few of the committers and contributors have been busy getting 3.0 ready for release.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] iakunin commented on pull request #10775: KAFKA-12668: Making MockScheduler.schedule safe to use in concurrent code

Posted by GitBox <gi...@apache.org>.
iakunin commented on pull request #10775:
URL: https://github.com/apache/kafka/pull/10775#issuecomment-884402579


   @jsancio got it. Thanks for explaining me that. I will wait patiently :)


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] iakunin edited a comment on pull request #10775: KAFKA-12668: Making MockScheduler.schedule safe to use in concurrent code

Posted by GitBox <gi...@apache.org>.
iakunin edited a comment on pull request #10775:
URL: https://github.com/apache/kafka/pull/10775#issuecomment-860042763






-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] iakunin commented on pull request #10775: KAFKA-12668: Making MockScheduler.schedule safe to use in concurrent code

Posted by GitBox <gi...@apache.org>.
iakunin commented on pull request #10775:
URL: https://github.com/apache/kafka/pull/10775#issuecomment-936453432


   @chia7712 @huxihx @jsancio hey guys! Any updates on this pull request? Maybe you have some time to have a look?


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] iakunin commented on pull request #10775: KAFKA-12668: Making MockScheduler.schedule safe to use in concurrent code

Posted by GitBox <gi...@apache.org>.
iakunin commented on pull request #10775:
URL: https://github.com/apache/kafka/pull/10775#issuecomment-883911555


   @chia7712 @huxihx could you have a look at this PR, please? Sorry for being impatient.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] mdedetrich commented on pull request #10775: KAFKA-12668: Making MockScheduler.schedule safe to use in concurrent code

Posted by GitBox <gi...@apache.org>.
mdedetrich commented on pull request #10775:
URL: https://github.com/apache/kafka/pull/10775#issuecomment-858574856


   It seems that the `scheduler.tick()` method is a workaround rather than solving the actual problem. From what I understand you are just suspending the scheduler and as a result of that the scheduler then hops back into the original background thread. If the `MockScheduler` changes the internal workings of task execution on threads then this can break the unspecified behavior?


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org