[GitHub] [kafka] chia7712 edited a comment on pull request #8657: KAFKA-8334 Make sure the thread which tries to complete delayed reque…

[GitBox <gi...@apache.org>]

chia7712 edited a comment on pull request #8657:
URL: https://github.com/apache/kafka/pull/8657#issuecomment-679855792


   > The second issue is that we hold a group lock while calling joinPurgatory.tryCompleteElseWatch. In this call, it's possible that DelayedJoin.onComplete() will be called. In that case, since the caller holds the group lock, we won't be completing partitionsToComplete in completeDelayedRequests().
   
   @junrao I go through group lock again and it is almost used anywhere :( 
   
   I'm worry about deadlock caused by current approach so I'd like to address @hachikuji refactor and your approach (separate thread). The following changes are included.
   
   1. ```Partition``` does not complete delayed operations. Instead, it adds those delayed operations to a queue. The callers ought to call the new method ```Partition.completeDelayedActions``` to complete those delayed operations in proper place (to avoid conflicting locking).
   1. apple above new method to all callers who need to complete delay operations.
   1. ```GroupCoordinator.onCompleteJoin``` is called by ```DelayedJoin``` only but it always held a group lock. To resolve it, we complete delayed requests in a separate thread. 
   1. keep using ```tryLock```. The known conflicting locks should be resolved by above changes. Using ```tryLock``` can protect us from deadlock which we have not noticed :)
   
   @junrao WDYT?


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