kerneltime commented on pull request #3091:
URL: https://github.com/apache/ozone/pull/3091#issuecomment-1086335457
@szetszwo I am not sure how this code is expected to work overall.
1. `ReferenceCountedDB` lives in the `ContainerCache` and currently the
logic to open and close a DB is tied to the lifecycle of the cache entry.
2. Can you point out the documentation recommending that the DB should be
closed on any exception.
3. The main problem we have in our code is that RocksDB expects that any
Java class that inherits `RocksObject` should be manually closed when done
instead of depending on the JVM calling the finalizer which then frees up the
memory allocated by the underlying JNI code. This code addresses some of the
code paths but there are other places where we need to reason about our memory
management. We currently do not have a leak but are depending on the JVM to
eventually clean up the memory.
4. The segfaults witnessed by the code were related to the open and close
path for RocksDB and this code change would not solve the problems we
encountered.
I think it is a worthwhile effort to fix our usage of RocksDB but we need to
complete the model including cache lifecycle management. If the underlying DB
is closed it should be evicted from the cache as well after the reference count
has gone down to 0. Also, if not all Exceptions need the DB to be closed, we
should then only close the DB for the Exceptions that cannot be recovered.
```
public enum Code {
Ok( (byte)0x0),
NotFound( (byte)0x1),
Corruption( (byte)0x2),
NotSupported( (byte)0x3),
InvalidArgument( (byte)0x4),
IOError( (byte)0x5),
MergeInProgress( (byte)0x6),
Incomplete( (byte)0x7),
ShutdownInProgress( (byte)0x8),
TimedOut( (byte)0x9),
Aborted( (byte)0xA),
Busy( (byte)0xB),
Expired( (byte)0xC),
TryAgain( (byte)0xD),
Undefined( (byte)0x7F);
```
NotFound shows up as an error code that is can be sent up as part of the
RocksDBException, is the normal key get impacted with closing the DB on
Exception?
```
public byte[] get(ColumnFamily family, byte[] key) throws IOException {
try {
return db.get(family.getHandle(), key);
} catch (RocksDBException e) {
close();
throw toIOException("get " + bytes2String(key) + " from " + family, e);
}
}
```
--
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]