keith-turner commented on a change in pull request #2213:
URL: https://github.com/apache/accumulo/pull/2213#discussion_r681926514
##########
File path:
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableImpl.java
##########
@@ -214,6 +202,9 @@ public CompactableImpl(Tablet tablet, CompactionManager
manager,
}
return Set.copyOf(servicesIds);
}, 2, TimeUnit.SECONDS);
+
+ this.fileMgr = new
TabletCompactionFileManager(Collections.unmodifiableSet(runningJobs),
+ tablet.getExtent(), extCompactingFiles, extSelInfo);
Review comment:
> I wish there was a better way, but not sure of what could work better.
@ctubbsii not sure if this workable, but one possible way is to move all in
memory state in compactableImpl into another class that handles all sync. A
lot of the code in compactableImpl has the following pattern.
```java
someMethod() {
synchronized(this) {
// examine/change multiple instance var that change the objects state
}
// do something like a metadata read/write or call to Tablet
synchronized(this) {
// examine/change multiple instance var that change the objects state
}
```
The above sync blocks could be replaced w/ calls to an object that
synchronized and manages all of the in memory state for CompactableImpl. The
changes in this PR take a subset of that in memory state (files) and move them
to an inner class.
Would need to analyze the code to see if its possible, but it could be one
way to simplify the code by grouping the in memory state and sync into a single
class (excluding the I/O type operations).
--
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]
