szetszwo commented on PR #3091: URL: https://github.com/apache/ozone/pull/3091#issuecomment-1088463382
> 4. The segfaults witnessed by the code ... @kerneltime , I agree that this change does not fix segfaults at all. This changes is to close the underlying resources. > 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. ... There may be possible improvements on cache management. However, it is outside the scope of this change. The current change is already not small. > ... 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. What exceptions are recoverable so that the db should not be closed? Could you give some examples? As shown in the example in https://github.com/facebook/rocksdb/wiki/RocksJava-Basics , they do suggest close the db for any exceptions. In the current code, if there is an exception, it is possible to lead to silent data loss since the db and other RocksObject are not closed. The usual exception handling is to close these objects so that the underlying files/objects can be closed and the underlying resources can be released. -- 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]
