mridulm commented on PR #36467:
URL: https://github.com/apache/spark/pull/36467#issuecomment-1123118193
> If the definition of iteratorTracker is changed from
ConcurrentLinkedQueue<Reference<RocksDBIterator<?>>> to
ConcurrentLinkedQueue<RocksDBIterator<?>> , then you are right.
It should not need change types.
In the current code, `ConcurrentLinkedQueue<Reference<RocksDBIterator<?>>>`
allows us to keep a reference to the `RocksDBIterator`, without holding a
strong reference to it (so it can be gc'ed if there are no other references to
it).
To sketch a possible solution:
```
RocksDBIterator:
public synchronized void close() throws IOException {
close(false);
}
// accessible from RocksDB
void close(boolean forceClose) {
if (db.notifyIteratorClosed(this) || forceClose) {
if (!closed) {
it.close();
closed = true;
}
}
}
RocksDB:
In close() : call it.close(true), instead.
For notifyIteratorClosed:
// return true if DB is still open
boolean notifyIteratorClosed(RocksDBIterator<?> it) {
iteratorTracker.removeIf(ref -> it.equals(ref.get()));
return _db.get() != null;
}
```
--
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]
