You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "keith-turner (via GitHub)" <gi...@apache.org> on 2023/05/27 16:50:16 UTC
[GitHub] [accumulo] keith-turner opened a new pull request, #3437: moves hostng request processing from tserver to manager
keith-turner opened a new pull request, #3437:
URL: https://github.com/apache/accumulo/pull/3437
Processing of hosting request was moved from the tserver to manager so that the tablet group watcher could be notified. This considerably reduces the delay in getting an ondemand tablet hosted.
Introduced a cache to avoid reprocessing hosting request for the same tablet in a short time period.
Modified the update of the hosting request column to use a conditional mutation. This avoids a metadata read before write (the read is now done by the mutation conditions) and avoids race conditions.
Moving the hosting request to the manager could introduce scaling problems, however the cache can help with this. The reduced latency for hosting ondemand tablet is needed, so if there are scaling problems a soluting will need to be found that has low latency for hosting.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #3437: moves hosting request processing from tserver to manager
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #3437:
URL: https://github.com/apache/accumulo/pull/3437#discussion_r1210143501
##########
server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java:
##########
@@ -89,13 +93,22 @@
import org.apache.zookeeper.KeeperException.NoNodeException;
import org.slf4j.Logger;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.Weigher;
+
public class ManagerClientServiceHandler implements ManagerClientService.Iface {
private static final Logger log = Manager.log;
private final Manager manager;
+ private final Cache<KeyExtent,Long> recentHostingRequest;
+
protected ManagerClientServiceHandler(Manager manager) {
this.manager = manager;
+ Weigher<KeyExtent,Long> weigher = (extent, t) -> Splitter.weigh(extent) + 8;
+ this.recentHostingRequest = Caffeine.newBuilder().expireAfterWrite(1, TimeUnit.MINUTES)
+ .maximumWeight(10_000_000L).weigher(weigher).build();
Review Comment:
Is this a max of 10M entries?
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #3437: moves hosting request processing from tserver to manager
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #3437:
URL: https://github.com/apache/accumulo/pull/3437#discussion_r1210280220
##########
server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java:
##########
@@ -89,13 +93,22 @@
import org.apache.zookeeper.KeeperException.NoNodeException;
import org.slf4j.Logger;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.Weigher;
+
public class ManagerClientServiceHandler implements ManagerClientService.Iface {
private static final Logger log = Manager.log;
private final Manager manager;
+ private final Cache<KeyExtent,Long> recentHostingRequest;
+
protected ManagerClientServiceHandler(Manager manager) {
this.manager = manager;
+ Weigher<KeyExtent,Long> weigher = (extent, t) -> Splitter.weigh(extent) + 8;
+ this.recentHostingRequest = Caffeine.newBuilder().expireAfterWrite(1, TimeUnit.MINUTES)
+ .maximumWeight(10_000_000L).weigher(weigher).build();
Review Comment:
Ok, I wasn't sure what the Weigher and maxWeight meant. I think it would be more clear if we did something like:
```
private static final long TEN_MB = 10 * 1024 * 1024;
```
and then use TEN_MB as the method parameter.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] keith-turner merged pull request #3437: moves hosting request processing from tserver to manager
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner merged PR #3437:
URL: https://github.com/apache/accumulo/pull/3437
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] keith-turner commented on a diff in pull request #3437: moves hosting request processing from tserver to manager
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3437:
URL: https://github.com/apache/accumulo/pull/3437#discussion_r1210289836
##########
server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java:
##########
@@ -89,13 +93,22 @@
import org.apache.zookeeper.KeeperException.NoNodeException;
import org.slf4j.Logger;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.Weigher;
+
public class ManagerClientServiceHandler implements ManagerClientService.Iface {
private static final Logger log = Manager.log;
private final Manager manager;
+ private final Cache<KeyExtent,Long> recentHostingRequest;
+
protected ManagerClientServiceHandler(Manager manager) {
this.manager = manager;
+ Weigher<KeyExtent,Long> weigher = (extent, t) -> Splitter.weigh(extent) + 8;
+ this.recentHostingRequest = Caffeine.newBuilder().expireAfterWrite(1, TimeUnit.MINUTES)
+ .maximumWeight(10_000_000L).weigher(weigher).build();
Review Comment:
updated in 7ea8d0b
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] keith-turner commented on a diff in pull request #3437: moves hosting request processing from tserver to manager
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3437:
URL: https://github.com/apache/accumulo/pull/3437#discussion_r1210250548
##########
server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java:
##########
@@ -89,13 +93,22 @@
import org.apache.zookeeper.KeeperException.NoNodeException;
import org.slf4j.Logger;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.Weigher;
+
public class ManagerClientServiceHandler implements ManagerClientService.Iface {
private static final Logger log = Manager.log;
private final Manager manager;
+ private final Cache<KeyExtent,Long> recentHostingRequest;
+
protected ManagerClientServiceHandler(Manager manager) {
this.manager = manager;
+ Weigher<KeyExtent,Long> weigher = (extent, t) -> Splitter.weigh(extent) + 8;
+ this.recentHostingRequest = Caffeine.newBuilder().expireAfterWrite(1, TimeUnit.MINUTES)
+ .maximumWeight(10_000_000L).weigher(weigher).build();
Review Comment:
Should be a max mem of 10M not including overhead of java objects.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org