comnetwork edited a comment on pull request #3899: URL: https://github.com/apache/hbase/pull/3899#issuecomment-986219358
@apache9，thank you very much for review. In my opinion: 1. It seems a reference count problem , the SnapshotSegmentScanner seems independent of whether snapshot segment itself is closed or not, even if snapshot segment is closed, if there is SnapshotSegmentScanner or SegmentScanner is using，we could not release the MemStoreLAB of the snapshot segment. 2. and I think that introducing another DelegateKeyValueScanner for wrapping SnapshotSegmentScanner is somewhat complex, because SnapshotSegmentScanner is only used for MemStoreSnapshot here, so seems that we create another additional wrapper for this snapshotSegmentScanner is somewhat complex and unnecessary because the lazy close logic overlaying reference count logic is much complex and error-prone, just use SnapshotSegmentScanner for reference count logic is clear and simple. 3. I find that MemStoreSnapshot.close is not always invoked under every situation, eg. flushing success or failed does not invoke MemStoreSnapshot.close in fact, which may be a bug, and I think we could remove the MemStoreSnapshot.close in the future to simplify the logic. 4. The overhead of creating a new SnapshotSegmentScanner every time when MemStoreSnapshot.getScanners is called is low, just as the behavior of DefaultMemStore.getScanners and Segment.getScanners.The key in the SnapshotSegmentScanner ctor is to increase the reference count of the MemStoreLAB of the snapshot segment. -- 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: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
