vikaskr22 commented on PR #6803: URL: https://github.com/apache/hadoop/pull/6803#issuecomment-2102594497
Hi @Hexiaoqiao , technically I didn't see or observe any issues till now but @ChenSammi has some concerns related to concurrency in other sub classes of AbstractDelegationTokenSecretManager.java . I had done testing with RangerKMS and there ZKDelegationTokenSecretManager.java is used as implementing class. That means, our test suite only verifies ZK specific cases. There are other implementation that is for HDFS federation, YARN etc. I don't have any test suite that ensures concurrency stability for other implementations as well. @ChenSammi point is, as part of last commit, I removed "synchronization" from super class and took care of concurrency in ZK sub class. So I was breaking concurrency at API level and all the implementing classes needs to take care of that. and at the same time, I don't have test suite internally that verifies anything except Ranger KMS DT usage. One more point she raised that, what about any other subclass outside this repo? They might have written their code in assumption that lock has been acquired at API level. Although I went through the source code of other implementing classes, and for me it still seems good. But due to lack of automated test suite and concurrency nature, I agreed to implement using Lock API. Using ReadWriteLock API, at least I am able to unblock multiple reader threads, like verifyToken(). Now verifyToken() can be invoked by multiple threads due to ReadLock nature. And write lock will ensure thread safety at API level. @ChenSammi , Please add your input as well in case I missed anything. 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
