divijvaidya commented on PR #13285: URL: https://github.com/apache/kafka/pull/13285#issuecomment-1443643050
Hello @chia7712 I have a question about this change. Please correct me if my understanding is wrong. In `SocketServer.scala`, `dataPlaneAcceptors` is a ConcurrentHashMap and hence, the keys could be be read/written-to by multiple threads. For `size()` computation, `ConcurrentHashMap` gives an approximate value which should suffice for metrics use case. However, the `processors` are stored in an `ArrayBuffer` which is mutable. The purpose of this lock on the entire SocketServer object is to ensure that `processors` are accessed in a thread safe manner. That is why we have a similar lock on entire SocketServer object in `acceptNewConnections()`, `removeProcessors()` etc. If we remove this lock, the `processors` will be accessed in a thread unsafe manner. Isn't that right? Perhaps a better way to solve this to 1\ reduce the granularity of the lock since we don't want a lock on entire SocketServer object but just on the processors and 2\ use shared locks and exclusive lock which will allow concurrent reads to happen and the same time entire that writes happen in isolation. -- 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.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
