Apache9 commented on pull request #3899: URL: https://github.com/apache/hbase/pull/3899#issuecomment-992636187
I do not think you fully got my point... My point is, the design of the MemStoreSnapshot is mainly for storing the snapshot id and the snapshot scanner, if we do not want to store the scanner in it, I think we could just remove this class, just call MemStore.getSnapshot every time when we want to use it... And for the problem 2, it is a problem, if it is a Closeable, then we should always close it. I checked the code, seems the only place where we miss the snapshot.close is when committing a flush, we should call close before calling updateStorefiles. And for problem 3, I do not see any difficulties to understand the logic? The design is that, the segment will not be changed any more, so we always use the same scanners to scan it. in order to not close it before we clear the snapshot, we should try to use DelegatingKeyValueScanner to implement an empty close method, and then close it in the MemStoreSnapshot.close method. This is an optimization, it will have more logics but I do not think it is too hard to understand. And for problem 4, as said above, this is an optimization, maybe it can not impact the performance a lot, but it does not introduce too much complexity too, so for me I think it is worth a try. There is a DelegatingKeyValueScanner implementation under the src/test, I think we can just move it src/main and use it. Thanks. -- 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]
