wchevreuil commented on a change in pull request #3389:
URL: https://github.com/apache/hbase/pull/3389#discussion_r672308643
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -694,7 +700,7 @@ private void
refreshStoreFilesInternal(Collection<StoreFileInfo> newFiles) throw
refreshStoreSizeAndTotalBytes();
}
- protected HStoreFile createStoreFileAndReader(final Path p) throws
IOException {
+ public HStoreFile createStoreFileAndReader(final Path p) throws IOException {
Review comment:
> Although, I also see that HStore#openStoreFiles is using one instance
of createStoreFileAndReader, so we couldn't completely move this out of HStore.
Is it better to lift one method into Store and leave other instances "internal"
to HStore? That might help the smell a little, but an obvious solution isn't
jumping out at me.
Yes, there are many other usages, pretty much everything that needs access
to hfiles would end up accessing this method (indirectly).
In terms of responsibility, it does seem right to me to have HStore the
central point for store file accesses. Moving it to Store would still require
some code duplication there, or exposing the two methods, which is what we are
trying to avoid here.
Maybe a better solution is to define a lambda function in
Compactor.commitCompaction, so that HStore can wrap up createStoreFileAndReader
when calling Compactor.commitCompaction().
--
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]
