sijie commented on issue #1284: Improve FileInfoBackingCache URL: https://github.com/apache/bookkeeper/pull/1284#issuecomment-375587424 > Ah yes. At minimum we should be checking again for existence inside the write lock. this is one of the change I made here, checking the existence. > But again, my question is whether this has been observed to be a problem? no at benchmark now. I hit the race condition then I noticed this behavior that file lookup is under a giant lock, which I would like to remove. because we have been working hard at SortedLedgerStorage to avoid doing any IO under lock. This FileInfoBackingCache unfortunately added this behavior back. So I have a strong concern about this and want to move the I/O operation out. However I don't want to change the locking behavior on getting FileInfo, reasons explained as below. > if we want to get rid of the locking, we should get rid of the big lock completely, which shouldn't be too hard with the concurrent map. a lot of race conditions can come around when competing on getting the fileinfo. so check-existence and put is much safer to happen under the write lock. and they are just memory operation, which is fine to happen under the write lock. However check if the file exists or not is an I/O operation, it is expensive, unpredictable than memory operation. so it should not happen under the lock. so the current approach is the most comfortable and safest approach I can think of. However if you really feel strong about this, feel free to start your approach.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
