kadirozde commented on code in PR #5545:
URL: https://github.com/apache/hbase/pull/5545#discussion_r1559993483
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreFileManager.java:
##########
@@ -86,13 +111,20 @@ public Collection<HStoreFile> getCompactedfiles() {
}
@Override
- public void insertNewFiles(Collection<HStoreFile> sfs) {
+ public void insertNewFiles(Collection<HStoreFile> sfs) throws IOException {
+ if (enableLiveFileTracking) {
+ this.liveStoreFiles =
ImmutableList.sortedCopyOf(getStoreFileComparator(),
Review Comment:
I just want to make sure that we are on the same page here. You think we
need to make this method atomic even though
(1) The class is not thread-safe as specified here
`Default implementation of StoreFileManager. Not thread-safe.`
(2) There is only one caller which is StoreEngine and it takes a write lock
whenever it calls a StoreFileManager methods, for example
```
public void addStoreFiles(Collection<HStoreFile> storeFiles,
IOExceptionRunnable actionAfterAdding) throws IOException {
storeFileTracker.add(StoreUtils.toStoreFileInfo(storeFiles));
writeLock();
try {
storeFileManager.insertNewFiles(storeFiles);
actionAfterAdding.run();
} finally {
// We need the lock, as long as we are updating the storeFiles
// or changing the memstore. Let us release it before calling
// notifyChangeReadersObservers. See HBASE-4485 for a possible
// deadlock scenario that could have happened if continue to hold
// the lock.
writeUnlock();
}
}
```
I can easily add a write lock here and to the other methods but given the
above points, is not it redundant?
--
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]
