ctubbsii commented on code in PR #2609:
URL: https://github.com/apache/accumulo/pull/2609#discussion_r848914162
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java:
##########
@@ -394,6 +402,23 @@ private static long jitter() {
* TabletServer.TIME_BETWEEN_LOCATOR_CACHE_CLEARS);
}
+ public Optional<Semaphore> getSemaphore() {
+ int configuredWriteThreadsMax =
+
getServerConfig().getConfiguration().getCount(Property.TSERV_WRITE_THREADS_MAX);
+ if (configuredWriteThreadsMax == MAX_WRITE_THREADS_DEFAULT) {
+ return Optional.empty();
+ } else {
+ synchronized (this) {
+ // if the value has changed, create a new semaphore
+ if (maxThreadPermits != configuredWriteThreadsMax) {
+ maxThreadPermits = configuredWriteThreadsMax;
+ writeThreadSemaphore = Optional.of(new Semaphore(maxThreadPermits));
Review Comment:
> So do you think we should only read the config at startup?
Possibly. If this were done as a thread pool, we could have it dynamically
resize without this problem. You might still be able to do it, by somehow
migrating the existing permits over, but that seems really complicated. If it's
done only once at startup, it simplifies things, but then it's made into a
thread pool later, it would go from a fixed config to a dynamic one, changing
behavior.
I think this is another argument for redesigning this as a thread pool
instead of proceeding with the Semaphore approach. I don't think the original
work was headed in the right direction with this approach.
--
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]
