showuon commented on code in PR #14483:
URL: https://github.com/apache/kafka/pull/14483#discussion_r1360248436
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/RemoteIndexCache.java:
##########
@@ -193,7 +192,16 @@ public File cacheDir() {
public void remove(Uuid key) {
lock.readLock().lock();
try {
- internalCache.invalidate(key);
+ internalCache.asMap().computeIfPresent(key, (k, v) -> {
+ try {
+ v.markForCleanup();
+ expiredIndexes.put(v);
Review Comment:
> I was just thinking around it , if this fails , it will create a dump of
files with delete suffix entry which never gets deleted from the disk ? Is the
behaviour ok ?
It's OK, it will be deleted in next startup. Of course we can have a way to
retry enqueue them, but I think it's a pretty rare case, we can keep it as is.
> During remove we try to take a readLock but in markForCleanUp we try to
take a write lock , Will it not result a deadlock state ?
The readlock in `remove` method is `RemoteIndexCache#lock`. The writeLock in
`markForCleanUp` method is `Entry#lock`. I agree it's confusing to have the
same name within the same parent class. Could you file another PR to update the
lock variable name?
--
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
