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
` /**
* Add the store files to store file manager, and also record it in the
store file tracker.
* <p/>
* The {@code actionAfterAdding} will be executed after the insertion to
store file manager, under
* the lock protection. Usually this is for clear the memstore snapshot.
*/
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]
