You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "dlmarion (via GitHub)" <gi...@apache.org> on 2024/02/27 17:32:22 UTC
[PR] Optimized logic for getting a random TabletServer connection [accumulo]
dlmarion opened a new pull request, #4309:
URL: https://github.com/apache/accumulo/pull/4309
The previous logic in this class would gather all of the Tserver ZNodes in ZooKeeper, then get the data for each ZNode and validate their ServiceLock. Then, after all of that it would randomly pick one of the TabletServers to connect to. It did this through the ZooCache object which on an initial connection would be empty and causes a lot of back and forth to ZooKeeper. The side effect of this is that the ZooCache would be populated with TabletServer information.
This change modifies the ClientContext to populate the ZooCache in a single-shot background task and modifies the default logic for getting a connection to a TabletServer. The new logic will make 2 calls to ZooKeeper in the best case scenario, one to get the list of TServer ZNodes in Zookeeper and another to get the ZNode data for one of them.
Fixes #4303
--
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
Re: [PR] Optimized logic for getting a random TabletServer connection [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on PR #4309:
URL: https://github.com/apache/accumulo/pull/4309#issuecomment-1969126905
> With the changes in this PR [this method](https://github.com/apache/accumulo/blob/c72bf8c00cb196d048d1aa35077327c55d6e58e2/core/src/main/java/org/apache/accumulo/core/clientImpl/ThriftTransportPool.java#L128) may only be called by test code now, so may be able to clean up and adjust the test code.
I got rid of the method in aecd790
--
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
Re: [PR] Optimized logic for getting a random TabletServer connection [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #4309:
URL: https://github.com/apache/accumulo/pull/4309#discussion_r1505158263
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java:
##########
@@ -249,6 +251,27 @@ public ClientContext(SingletonReservation reservation, ClientInfo info,
clientThreadPools = ThreadPools.getClientThreadPools(ueh);
}
}
+ // Kick off a task to try and populate the ZooCache with TabletServer
+ // information. It may not be complete as the cluster may be starting.
+ // It's a best effort
+ clientThreadPools
Review Comment:
> You are suggesting that we don't need to preload ZooCache?
Thinking in a client instance that is busy that eventually the cache will be populated. On a client that is not busy the cache will not be populated, but thats ok it does not need the data.
> Do you think there is a downside to removing this?
There may be a downside, but can not think of something specific. Thinking we are not sure if there is an upside and the behavior could cause uneeded ZK load, so maybe its ok to omit.
--
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
Re: [PR] Optimized logic for getting a random TabletServer connection [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #4309:
URL: https://github.com/apache/accumulo/pull/4309#discussion_r1507749408
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ThriftTransportKey.java:
##########
@@ -81,7 +92,7 @@ public boolean equals(Object o) {
return false;
}
ThriftTransportKey ttk = (ThriftTransportKey) o;
- return server.equals(ttk.server) && timeout == ttk.timeout
+ return type.equals(ttk.type) && server.equals(ttk.server) && timeout == ttk.timeout
Review Comment:
Looked at ThriftClientTypes and did not see that it implements equals() or hashCode(). Seems like the intention is that there is only one instance of type in the JVM so its ok to fall back to the object hashCode. This makes it tricky to reason about the correctness, not sure of a way to improve this. If possible it would be good to reduce/localize the amount of assumptions being made around this. Thought through the following possible solutions, but they seemed like dead ends.
* could we make ThriftClientTypes an enum? Does not seem like this would work with the type parameter.
* could we make ThriftClientTypes constructor private? This makes it easier to reason about single instances. The subclasses would probably need to be pulled in as inner classes to do this. Not sure if its workable even then though.
* could we add equals and hashCode impls that only look at the service name? This feels dubious because it excludes the factory
This issue does not need to hold up this PR if the current code does only create a single instance per type in the JVM. Did not attempt to verify if this is the case or not.
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -57,44 +59,56 @@ default Pair<String,C> getTabletServerConnection(Logger LOG, ThriftClientTypes<C
ClientContext context, boolean preferCachedConnections, AtomicBoolean warned)
throws TTransportException {
checkArgument(context != null, "context is null");
- long rpcTimeout = context.getClientTimeoutInMillis();
- // create list of servers
- ArrayList<ThriftTransportKey> servers = new ArrayList<>();
- // add tservers
- ZooCache zc = context.getZooCache();
- for (String tserver : zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS)) {
+ if (preferCachedConnections) {
+ Pair<String,TTransport> cachedTransport =
+ context.getTransportPool().getAnyCachedTransport(type);
+ if (cachedTransport != null) {
+ C client = ThriftUtil.createClient(type, cachedTransport.getSecond());
+ warned.set(false);
+ return new Pair<String,C>(cachedTransport.getFirst(), client);
+ }
+ }
+
+ final long rpcTimeout = context.getClientTimeoutInMillis();
+ final ZooCache zc = context.getZooCache();
+ final List<String> tservers = new ArrayList<>();
+
+ tservers.addAll(zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS));
+
+ if (tservers.isEmpty()) {
+ if (warned.compareAndSet(false, true)) {
+ LOG.warn("There are no tablet servers: check that zookeeper and accumulo are running.");
+ }
+ throw new TTransportException("There are no servers for type: " + type);
+ }
+
+ // Try to connect to an online tserver
+ Collections.shuffle(tservers);
+ for (String tserver : tservers) {
var zLocPath =
ServiceLock.path(context.getZooKeeperRoot() + Constants.ZTSERVERS + "/" + tserver);
byte[] data = zc.getLockData(zLocPath);
if (data != null) {
String strData = new String(data, UTF_8);
if (!strData.equals("manager")) {
- servers.add(new ThriftTransportKey(
- new ServerServices(strData).getAddress(Service.TSERV_CLIENT), rpcTimeout, context));
- }
- }
- }
-
- boolean opened = false;
- try {
- Pair<String,TTransport> pair =
- context.getTransportPool().getAnyTransport(servers, preferCachedConnections);
- C client = ThriftUtil.createClient(type, pair.getSecond());
- opened = true;
- warned.set(false);
- return new Pair<>(pair.getFirst(), client);
- } finally {
- if (!opened) {
- if (warned.compareAndSet(false, true)) {
- if (servers.isEmpty()) {
- LOG.warn("There are no tablet servers: check that zookeeper and accumulo are running.");
- } else {
- LOG.warn("Failed to find an available server in the list of servers: {}", servers);
+ final HostAndPort tserverClientAddress =
+ new ServerServices(strData).getAddress(Service.TSERV_CLIENT);
+ try {
+ TTransport transport = context.getTransportPool().getTransport(type,
+ tserverClientAddress, rpcTimeout, context, preferCachedConnections);
+ C client = ThriftUtil.createClient(type, transport);
+ warned.set(false);
+ return new Pair<String,C>(tserverClientAddress.toString(), client);
+ } catch (TTransportException e) {
+ LOG.trace("Error creating transport to {}", tserverClientAddress);
+ continue;
}
}
}
}
+ LOG.warn("Failed to find an available server in the list of servers: {}", tservers);
Review Comment:
Maybe this should use the warned atomic bool. Also could include the type in the log message.
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -57,44 +59,56 @@ default Pair<String,C> getTabletServerConnection(Logger LOG, ThriftClientTypes<C
ClientContext context, boolean preferCachedConnections, AtomicBoolean warned)
throws TTransportException {
checkArgument(context != null, "context is null");
- long rpcTimeout = context.getClientTimeoutInMillis();
- // create list of servers
- ArrayList<ThriftTransportKey> servers = new ArrayList<>();
- // add tservers
- ZooCache zc = context.getZooCache();
- for (String tserver : zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS)) {
+ if (preferCachedConnections) {
+ Pair<String,TTransport> cachedTransport =
+ context.getTransportPool().getAnyCachedTransport(type);
+ if (cachedTransport != null) {
+ C client = ThriftUtil.createClient(type, cachedTransport.getSecond());
+ warned.set(false);
+ return new Pair<String,C>(cachedTransport.getFirst(), client);
+ }
+ }
+
+ final long rpcTimeout = context.getClientTimeoutInMillis();
+ final ZooCache zc = context.getZooCache();
+ final List<String> tservers = new ArrayList<>();
+
+ tservers.addAll(zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS));
+
+ if (tservers.isEmpty()) {
+ if (warned.compareAndSet(false, true)) {
+ LOG.warn("There are no tablet servers: check that zookeeper and accumulo are running.");
Review Comment:
Is this code used for more than tservers now? Should the log message replace `tablet servers` with the type?
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -57,44 +59,56 @@ default Pair<String,C> getTabletServerConnection(Logger LOG, ThriftClientTypes<C
ClientContext context, boolean preferCachedConnections, AtomicBoolean warned)
throws TTransportException {
checkArgument(context != null, "context is null");
- long rpcTimeout = context.getClientTimeoutInMillis();
- // create list of servers
- ArrayList<ThriftTransportKey> servers = new ArrayList<>();
- // add tservers
- ZooCache zc = context.getZooCache();
- for (String tserver : zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS)) {
+ if (preferCachedConnections) {
+ Pair<String,TTransport> cachedTransport =
+ context.getTransportPool().getAnyCachedTransport(type);
+ if (cachedTransport != null) {
+ C client = ThriftUtil.createClient(type, cachedTransport.getSecond());
+ warned.set(false);
+ return new Pair<String,C>(cachedTransport.getFirst(), client);
+ }
+ }
+
+ final long rpcTimeout = context.getClientTimeoutInMillis();
+ final ZooCache zc = context.getZooCache();
+ final List<String> tservers = new ArrayList<>();
+
+ tservers.addAll(zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS));
+
+ if (tservers.isEmpty()) {
+ if (warned.compareAndSet(false, true)) {
+ LOG.warn("There are no tablet servers: check that zookeeper and accumulo are running.");
+ }
+ throw new TTransportException("There are no servers for type: " + type);
+ }
+
+ // Try to connect to an online tserver
+ Collections.shuffle(tservers);
+ for (String tserver : tservers) {
var zLocPath =
ServiceLock.path(context.getZooKeeperRoot() + Constants.ZTSERVERS + "/" + tserver);
byte[] data = zc.getLockData(zLocPath);
if (data != null) {
String strData = new String(data, UTF_8);
if (!strData.equals("manager")) {
- servers.add(new ThriftTransportKey(
- new ServerServices(strData).getAddress(Service.TSERV_CLIENT), rpcTimeout, context));
- }
- }
- }
-
- boolean opened = false;
- try {
- Pair<String,TTransport> pair =
- context.getTransportPool().getAnyTransport(servers, preferCachedConnections);
- C client = ThriftUtil.createClient(type, pair.getSecond());
- opened = true;
- warned.set(false);
- return new Pair<>(pair.getFirst(), client);
- } finally {
- if (!opened) {
- if (warned.compareAndSet(false, true)) {
- if (servers.isEmpty()) {
- LOG.warn("There are no tablet servers: check that zookeeper and accumulo are running.");
- } else {
- LOG.warn("Failed to find an available server in the list of servers: {}", servers);
+ final HostAndPort tserverClientAddress =
+ new ServerServices(strData).getAddress(Service.TSERV_CLIENT);
+ try {
+ TTransport transport = context.getTransportPool().getTransport(type,
+ tserverClientAddress, rpcTimeout, context, preferCachedConnections);
+ C client = ThriftUtil.createClient(type, transport);
+ warned.set(false);
+ return new Pair<String,C>(tserverClientAddress.toString(), client);
+ } catch (TTransportException e) {
+ LOG.trace("Error creating transport to {}", tserverClientAddress);
+ continue;
}
}
}
}
+ LOG.warn("Failed to find an available server in the list of servers: {}", tservers);
+ throw new TTransportException("Failed to connect to any server");
Review Comment:
could include type in the message
##########
core/src/test/java/org/apache/accumulo/core/clientImpl/ThriftTransportKeyTest.java:
##########
@@ -136,8 +138,8 @@ public void testSimpleEquivalence() {
replay(clientCtx);
- ThriftTransportKey ttk =
- new ThriftTransportKey(HostAndPort.fromParts("localhost", 9999), 120_000, clientCtx);
+ ThriftTransportKey ttk = new ThriftTransportKey(ThriftClientTypes.CLIENT,
+ HostAndPort.fromParts("localhost", 9999), 120_000, clientCtx);
assertEquals(ttk, ttk, "Normal ThriftTransportKey doesn't equal itself");
Review Comment:
This is a preexisting problem with this test, would be nice to make it follow the pattern in the other existing test.
```suggestion
assertEquals(ttk, ttk, "Normal ThriftTransportKey doesn't equal itself");
assertEquals(ttk.hashCode(), ttk.hashCode());
```
--
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
Re: [PR] Optimized logic for getting a random TabletServer connection [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #4309:
URL: https://github.com/apache/accumulo/pull/4309#discussion_r1505011844
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -57,44 +60,54 @@ default Pair<String,C> getTabletServerConnection(Logger LOG, ThriftClientTypes<C
ClientContext context, boolean preferCachedConnections, AtomicBoolean warned)
throws TTransportException {
checkArgument(context != null, "context is null");
- long rpcTimeout = context.getClientTimeoutInMillis();
- // create list of servers
- ArrayList<ThriftTransportKey> servers = new ArrayList<>();
+ final long rpcTimeout = context.getClientTimeoutInMillis();
- // add tservers
- ZooCache zc = context.getZooCache();
- for (String tserver : zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS)) {
- var zLocPath =
- ServiceLock.path(context.getZooKeeperRoot() + Constants.ZTSERVERS + "/" + tserver);
- byte[] data = zc.getLockData(zLocPath);
- if (data != null) {
- String strData = new String(data, UTF_8);
- if (!strData.equals("manager")) {
- servers.add(new ThriftTransportKey(
- new ServerServices(strData).getAddress(Service.TSERV_CLIENT), rpcTimeout, context));
+ final ZooCache zc = context.getZooCache();
+ final List<String> tservers = new ArrayList<>();
+ final AtomicBoolean warnedAboutTServersBeingDown = new AtomicBoolean(false);
+
+ for (int retries = 0; retries < 10; retries++) {
Review Comment:
The existing code did not retry and poking around it seems that code that calls this method does retry at higher levels. It would probably be better to not retry here and defer to the calling code. Although I don't fully understand the overall behavior of the existing code when the set of tservers in ZK is empty, sitll trying to understand that. Not exaclty sure how the existing code would retry. It seems like the addition of throwing TTransportException in this PR will help with the higher level code that retries, but not sure how it worked before these changes.
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -57,44 +60,54 @@ default Pair<String,C> getTabletServerConnection(Logger LOG, ThriftClientTypes<C
ClientContext context, boolean preferCachedConnections, AtomicBoolean warned)
throws TTransportException {
checkArgument(context != null, "context is null");
- long rpcTimeout = context.getClientTimeoutInMillis();
- // create list of servers
- ArrayList<ThriftTransportKey> servers = new ArrayList<>();
+ final long rpcTimeout = context.getClientTimeoutInMillis();
- // add tservers
- ZooCache zc = context.getZooCache();
- for (String tserver : zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS)) {
- var zLocPath =
- ServiceLock.path(context.getZooKeeperRoot() + Constants.ZTSERVERS + "/" + tserver);
- byte[] data = zc.getLockData(zLocPath);
- if (data != null) {
- String strData = new String(data, UTF_8);
- if (!strData.equals("manager")) {
- servers.add(new ThriftTransportKey(
- new ServerServices(strData).getAddress(Service.TSERV_CLIENT), rpcTimeout, context));
+ final ZooCache zc = context.getZooCache();
+ final List<String> tservers = new ArrayList<>();
+ final AtomicBoolean warnedAboutTServersBeingDown = new AtomicBoolean(false);
+
+ for (int retries = 0; retries < 10; retries++) {
+ // Cluster may not be up, wait for tservers to come online
+ while (true) {
+ tservers.addAll(zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS));
+
+ if (!tservers.isEmpty()) {
+ break;
}
+
+ if (tservers.isEmpty() && !warnedAboutTServersBeingDown.get()) {
+ LOG.warn("There are no tablet servers: check that zookeeper and accumulo are running.");
+ warnedAboutTServersBeingDown.set(true);
+ }
+ UtilWaitThread.sleep(100);
}
- }
- boolean opened = false;
- try {
- Pair<String,TTransport> pair =
- context.getTransportPool().getAnyTransport(servers, preferCachedConnections);
- C client = ThriftUtil.createClient(type, pair.getSecond());
- opened = true;
- warned.set(false);
- return new Pair<>(pair.getFirst(), client);
- } finally {
- if (!opened) {
- if (warned.compareAndSet(false, true)) {
- if (servers.isEmpty()) {
- LOG.warn("There are no tablet servers: check that zookeeper and accumulo are running.");
- } else {
- LOG.warn("Failed to find an available server in the list of servers: {}", servers);
+ // Try to connect to an online tserver
+ Collections.shuffle(tservers);
+ for (String tserver : tservers) {
+ var zLocPath =
+ ServiceLock.path(context.getZooKeeperRoot() + Constants.ZTSERVERS + "/" + tserver);
+ byte[] data = zc.getLockData(zLocPath);
+ if (data != null) {
+ String strData = new String(data, UTF_8);
+ if (!strData.equals("manager")) {
Review Comment:
This is a pre-existing problem unrelated to this PR. Is there a constant we could use here instead of the literal `"manager"`? I took a quick look around and did not find anything. I can open an issue about this if needed.
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -57,44 +60,54 @@ default Pair<String,C> getTabletServerConnection(Logger LOG, ThriftClientTypes<C
ClientContext context, boolean preferCachedConnections, AtomicBoolean warned)
throws TTransportException {
checkArgument(context != null, "context is null");
- long rpcTimeout = context.getClientTimeoutInMillis();
- // create list of servers
- ArrayList<ThriftTransportKey> servers = new ArrayList<>();
+ final long rpcTimeout = context.getClientTimeoutInMillis();
- // add tservers
- ZooCache zc = context.getZooCache();
- for (String tserver : zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS)) {
- var zLocPath =
- ServiceLock.path(context.getZooKeeperRoot() + Constants.ZTSERVERS + "/" + tserver);
- byte[] data = zc.getLockData(zLocPath);
- if (data != null) {
- String strData = new String(data, UTF_8);
- if (!strData.equals("manager")) {
- servers.add(new ThriftTransportKey(
- new ServerServices(strData).getAddress(Service.TSERV_CLIENT), rpcTimeout, context));
+ final ZooCache zc = context.getZooCache();
+ final List<String> tservers = new ArrayList<>();
+ final AtomicBoolean warnedAboutTServersBeingDown = new AtomicBoolean(false);
Review Comment:
There is a method argument passed in named `warned`, it should probably be used instead. I think that variable is used among many threads and prevents lots of threads from separately attempting to log the same warning.
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -57,44 +60,54 @@ default Pair<String,C> getTabletServerConnection(Logger LOG, ThriftClientTypes<C
ClientContext context, boolean preferCachedConnections, AtomicBoolean warned)
throws TTransportException {
checkArgument(context != null, "context is null");
- long rpcTimeout = context.getClientTimeoutInMillis();
- // create list of servers
- ArrayList<ThriftTransportKey> servers = new ArrayList<>();
+ final long rpcTimeout = context.getClientTimeoutInMillis();
Review Comment:
The preferCachedConnections argument is no longer being used. Maybe we could do something like the following
```suggestion
if(preferCachedConnections) {
Optional<Pair<String,TTransport>> cachedTransport = context.getTransportPool().getAnyCachedTransport();
if(cachedTransport.isPresent()) {
// return the cached transport
}
}
final long rpcTimeout = context.getClientTimeoutInMillis();
```
Where` getAnyCachedTransport()` would be a new method on ThriftTransportPool that reuses [this code ](https://github.com/apache/accumulo/blob/c72bf8c00cb196d048d1aa35077327c55d6e58e2/core/src/main/java/org/apache/accumulo/core/clientImpl/ThriftTransportPool.java#L137-L152) with some modifications not to do the set intersection. The new method could find and reserve an unreserved cached transport for a random server if one is available.
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java:
##########
@@ -249,6 +251,27 @@ public ClientContext(SingletonReservation reservation, ClientInfo info,
clientThreadPools = ThreadPools.getClientThreadPools(ueh);
}
}
+ // Kick off a task to try and populate the ZooCache with TabletServer
+ // information. It may not be complete as the cluster may be starting.
+ // It's a best effort
+ clientThreadPools
Review Comment:
I don't think we should add this as it will add load on ZK that may not be worth the effort depending on what the accumulo client does and how long it lives. For example lots of accumulo clients created by map reduce or spark jobs will probably not benefit from populating this cache, but when they all start willl generate a lot of uneeded ZK activity. The other changes in this PR to TServerClient could really improve that scenario. The changes in this PR to TServerClient are really nice from the perspective of reducing ZK load when lots of Accumulo client processes start around the same time.
--
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
Re: [PR] Optimized logic for getting a random TabletServer connection [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #4309:
URL: https://github.com/apache/accumulo/pull/4309#discussion_r1505017993
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -57,44 +60,54 @@ default Pair<String,C> getTabletServerConnection(Logger LOG, ThriftClientTypes<C
ClientContext context, boolean preferCachedConnections, AtomicBoolean warned)
throws TTransportException {
checkArgument(context != null, "context is null");
- long rpcTimeout = context.getClientTimeoutInMillis();
- // create list of servers
- ArrayList<ThriftTransportKey> servers = new ArrayList<>();
+ final long rpcTimeout = context.getClientTimeoutInMillis();
- // add tservers
- ZooCache zc = context.getZooCache();
- for (String tserver : zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS)) {
- var zLocPath =
- ServiceLock.path(context.getZooKeeperRoot() + Constants.ZTSERVERS + "/" + tserver);
- byte[] data = zc.getLockData(zLocPath);
- if (data != null) {
- String strData = new String(data, UTF_8);
- if (!strData.equals("manager")) {
- servers.add(new ThriftTransportKey(
- new ServerServices(strData).getAddress(Service.TSERV_CLIENT), rpcTimeout, context));
+ final ZooCache zc = context.getZooCache();
+ final List<String> tservers = new ArrayList<>();
+ final AtomicBoolean warnedAboutTServersBeingDown = new AtomicBoolean(false);
+
+ for (int retries = 0; retries < 10; retries++) {
Review Comment:
This is [one example](https://github.com/apache/accumulo/blob/c72bf8c00cb196d048d1aa35077327c55d6e58e2/core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java#L100-L125) of code I was looking at where retry happens at a higher level that calls this method indirectly.
--
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
Re: [PR] Optimized logic for getting a random TabletServer connection [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4309:
URL: https://github.com/apache/accumulo/pull/4309#discussion_r1506071724
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -57,44 +60,54 @@ default Pair<String,C> getTabletServerConnection(Logger LOG, ThriftClientTypes<C
ClientContext context, boolean preferCachedConnections, AtomicBoolean warned)
throws TTransportException {
checkArgument(context != null, "context is null");
- long rpcTimeout = context.getClientTimeoutInMillis();
- // create list of servers
- ArrayList<ThriftTransportKey> servers = new ArrayList<>();
+ final long rpcTimeout = context.getClientTimeoutInMillis();
Review Comment:
I'm using the preferCachedConnections parameter and implemented something in ThriftTransportPool in in aecd790
--
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
Re: [PR] Optimized logic for getting a random TabletServer connection [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4309:
URL: https://github.com/apache/accumulo/pull/4309#discussion_r1506070505
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -57,44 +60,54 @@ default Pair<String,C> getTabletServerConnection(Logger LOG, ThriftClientTypes<C
ClientContext context, boolean preferCachedConnections, AtomicBoolean warned)
throws TTransportException {
checkArgument(context != null, "context is null");
- long rpcTimeout = context.getClientTimeoutInMillis();
- // create list of servers
- ArrayList<ThriftTransportKey> servers = new ArrayList<>();
+ final long rpcTimeout = context.getClientTimeoutInMillis();
- // add tservers
- ZooCache zc = context.getZooCache();
- for (String tserver : zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS)) {
- var zLocPath =
- ServiceLock.path(context.getZooKeeperRoot() + Constants.ZTSERVERS + "/" + tserver);
- byte[] data = zc.getLockData(zLocPath);
- if (data != null) {
- String strData = new String(data, UTF_8);
- if (!strData.equals("manager")) {
- servers.add(new ThriftTransportKey(
- new ServerServices(strData).getAddress(Service.TSERV_CLIENT), rpcTimeout, context));
+ final ZooCache zc = context.getZooCache();
+ final List<String> tservers = new ArrayList<>();
+ final AtomicBoolean warnedAboutTServersBeingDown = new AtomicBoolean(false);
Review Comment:
Good catch. I used the warn variable in aecd790
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java:
##########
@@ -249,6 +251,27 @@ public ClientContext(SingletonReservation reservation, ClientInfo info,
clientThreadPools = ThreadPools.getClientThreadPools(ueh);
}
}
+ // Kick off a task to try and populate the ZooCache with TabletServer
+ // information. It may not be complete as the cluster may be starting.
+ // It's a best effort
+ clientThreadPools
Review Comment:
I removed this in in aecd790
--
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
Re: [PR] Optimized logic for getting a random TabletServer connection [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4309:
URL: https://github.com/apache/accumulo/pull/4309#discussion_r1509130716
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ThriftTransportKey.java:
##########
@@ -81,7 +92,7 @@ public boolean equals(Object o) {
return false;
}
ThriftTransportKey ttk = (ThriftTransportKey) o;
- return server.equals(ttk.server) && timeout == ttk.timeout
+ return type.equals(ttk.type) && server.equals(ttk.server) && timeout == ttk.timeout
Review Comment:
I could not make ThriftClientTypes constructor package protected. The ThriftProcessorTypes class in server-base needs access. I was able to make it `protected` however in 1a3d5e0
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -57,44 +59,56 @@ default Pair<String,C> getTabletServerConnection(Logger LOG, ThriftClientTypes<C
ClientContext context, boolean preferCachedConnections, AtomicBoolean warned)
throws TTransportException {
checkArgument(context != null, "context is null");
- long rpcTimeout = context.getClientTimeoutInMillis();
- // create list of servers
- ArrayList<ThriftTransportKey> servers = new ArrayList<>();
- // add tservers
- ZooCache zc = context.getZooCache();
- for (String tserver : zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS)) {
+ if (preferCachedConnections) {
+ Pair<String,TTransport> cachedTransport =
+ context.getTransportPool().getAnyCachedTransport(type);
+ if (cachedTransport != null) {
+ C client = ThriftUtil.createClient(type, cachedTransport.getSecond());
+ warned.set(false);
+ return new Pair<String,C>(cachedTransport.getFirst(), client);
+ }
+ }
+
+ final long rpcTimeout = context.getClientTimeoutInMillis();
+ final ZooCache zc = context.getZooCache();
+ final List<String> tservers = new ArrayList<>();
+
+ tservers.addAll(zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS));
+
+ if (tservers.isEmpty()) {
+ if (warned.compareAndSet(false, true)) {
+ LOG.warn("There are no tablet servers: check that zookeeper and accumulo are running.");
+ }
+ throw new TTransportException("There are no servers for type: " + type);
+ }
+
+ // Try to connect to an online tserver
+ Collections.shuffle(tservers);
+ for (String tserver : tservers) {
var zLocPath =
ServiceLock.path(context.getZooKeeperRoot() + Constants.ZTSERVERS + "/" + tserver);
byte[] data = zc.getLockData(zLocPath);
if (data != null) {
String strData = new String(data, UTF_8);
if (!strData.equals("manager")) {
- servers.add(new ThriftTransportKey(
- new ServerServices(strData).getAddress(Service.TSERV_CLIENT), rpcTimeout, context));
- }
- }
- }
-
- boolean opened = false;
- try {
- Pair<String,TTransport> pair =
- context.getTransportPool().getAnyTransport(servers, preferCachedConnections);
- C client = ThriftUtil.createClient(type, pair.getSecond());
- opened = true;
- warned.set(false);
- return new Pair<>(pair.getFirst(), client);
- } finally {
- if (!opened) {
- if (warned.compareAndSet(false, true)) {
- if (servers.isEmpty()) {
- LOG.warn("There are no tablet servers: check that zookeeper and accumulo are running.");
- } else {
- LOG.warn("Failed to find an available server in the list of servers: {}", servers);
+ final HostAndPort tserverClientAddress =
+ new ServerServices(strData).getAddress(Service.TSERV_CLIENT);
+ try {
+ TTransport transport = context.getTransportPool().getTransport(type,
+ tserverClientAddress, rpcTimeout, context, preferCachedConnections);
+ C client = ThriftUtil.createClient(type, transport);
+ warned.set(false);
+ return new Pair<String,C>(tserverClientAddress.toString(), client);
+ } catch (TTransportException e) {
+ LOG.trace("Error creating transport to {}", tserverClientAddress);
+ continue;
}
}
}
}
+ LOG.warn("Failed to find an available server in the list of servers: {}", tservers);
Review Comment:
Added this in 1a3d5e0
--
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
Re: [PR] Optimized logic for getting a random TabletServer connection [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4309:
URL: https://github.com/apache/accumulo/pull/4309#discussion_r1505087227
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java:
##########
@@ -249,6 +251,27 @@ public ClientContext(SingletonReservation reservation, ClientInfo info,
clientThreadPools = ThreadPools.getClientThreadPools(ueh);
}
}
+ // Kick off a task to try and populate the ZooCache with TabletServer
+ // information. It may not be complete as the cluster may be starting.
+ // It's a best effort
+ clientThreadPools
Review Comment:
I have no issue removing this, I was just trying to keep the same behavior - the current code loads ZooCache on first connection, which is the cause of the delay. You are suggesting that we don't need to preload ZooCache? Do you think there is a downside to removing this?
--
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
Re: [PR] Optimized logic for getting a random TabletServer connection [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4309:
URL: https://github.com/apache/accumulo/pull/4309#discussion_r1506069659
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -57,44 +60,54 @@ default Pair<String,C> getTabletServerConnection(Logger LOG, ThriftClientTypes<C
ClientContext context, boolean preferCachedConnections, AtomicBoolean warned)
throws TTransportException {
checkArgument(context != null, "context is null");
- long rpcTimeout = context.getClientTimeoutInMillis();
- // create list of servers
- ArrayList<ThriftTransportKey> servers = new ArrayList<>();
+ final long rpcTimeout = context.getClientTimeoutInMillis();
- // add tservers
- ZooCache zc = context.getZooCache();
- for (String tserver : zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS)) {
- var zLocPath =
- ServiceLock.path(context.getZooKeeperRoot() + Constants.ZTSERVERS + "/" + tserver);
- byte[] data = zc.getLockData(zLocPath);
- if (data != null) {
- String strData = new String(data, UTF_8);
- if (!strData.equals("manager")) {
- servers.add(new ThriftTransportKey(
- new ServerServices(strData).getAddress(Service.TSERV_CLIENT), rpcTimeout, context));
+ final ZooCache zc = context.getZooCache();
+ final List<String> tservers = new ArrayList<>();
+ final AtomicBoolean warnedAboutTServersBeingDown = new AtomicBoolean(false);
+
+ for (int retries = 0; retries < 10; retries++) {
Review Comment:
I removed the retry and while loop in aecd790
--
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
Re: [PR] Optimized logic for getting a random TabletServer connection [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4309:
URL: https://github.com/apache/accumulo/pull/4309#discussion_r1508934819
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ThriftTransportKey.java:
##########
@@ -81,7 +92,7 @@ public boolean equals(Object o) {
return false;
}
ThriftTransportKey ttk = (ThriftTransportKey) o;
- return server.equals(ttk.server) && timeout == ttk.timeout
+ return type.equals(ttk.type) && server.equals(ttk.server) && timeout == ttk.timeout
Review Comment:
> could we make ThriftClientTypes an enum? Does not seem like this would work with the type parameter.
IIRC I tried this when I originally created ThriftClientTypes and it didn't work with the generics
> could we make ThriftClientTypes constructor private? This makes it easier to reason about single instances. The subclasses would probably need to be pulled in as inner classes to do this. Not sure if its workable even then though.
I think we could probably make it `protected` vs `public`, or remove the modifier and make it package protected. I looked at all of the impls and their constructors have no modifier, so they are package protected.
--
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
Re: [PR] Optimized logic for getting a random TabletServer connection [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4309:
URL: https://github.com/apache/accumulo/pull/4309#discussion_r1509131216
##########
core/src/test/java/org/apache/accumulo/core/clientImpl/ThriftTransportKeyTest.java:
##########
@@ -136,8 +138,8 @@ public void testSimpleEquivalence() {
replay(clientCtx);
- ThriftTransportKey ttk =
- new ThriftTransportKey(HostAndPort.fromParts("localhost", 9999), 120_000, clientCtx);
+ ThriftTransportKey ttk = new ThriftTransportKey(ThriftClientTypes.CLIENT,
+ HostAndPort.fromParts("localhost", 9999), 120_000, clientCtx);
assertEquals(ttk, ttk, "Normal ThriftTransportKey doesn't equal itself");
Review Comment:
Added this in 1a3d5e0
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -57,44 +59,56 @@ default Pair<String,C> getTabletServerConnection(Logger LOG, ThriftClientTypes<C
ClientContext context, boolean preferCachedConnections, AtomicBoolean warned)
throws TTransportException {
checkArgument(context != null, "context is null");
- long rpcTimeout = context.getClientTimeoutInMillis();
- // create list of servers
- ArrayList<ThriftTransportKey> servers = new ArrayList<>();
- // add tservers
- ZooCache zc = context.getZooCache();
- for (String tserver : zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS)) {
+ if (preferCachedConnections) {
+ Pair<String,TTransport> cachedTransport =
+ context.getTransportPool().getAnyCachedTransport(type);
+ if (cachedTransport != null) {
+ C client = ThriftUtil.createClient(type, cachedTransport.getSecond());
+ warned.set(false);
+ return new Pair<String,C>(cachedTransport.getFirst(), client);
+ }
+ }
+
+ final long rpcTimeout = context.getClientTimeoutInMillis();
+ final ZooCache zc = context.getZooCache();
+ final List<String> tservers = new ArrayList<>();
+
+ tservers.addAll(zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS));
+
+ if (tservers.isEmpty()) {
+ if (warned.compareAndSet(false, true)) {
+ LOG.warn("There are no tablet servers: check that zookeeper and accumulo are running.");
+ }
+ throw new TTransportException("There are no servers for type: " + type);
+ }
+
+ // Try to connect to an online tserver
+ Collections.shuffle(tservers);
+ for (String tserver : tservers) {
var zLocPath =
ServiceLock.path(context.getZooKeeperRoot() + Constants.ZTSERVERS + "/" + tserver);
byte[] data = zc.getLockData(zLocPath);
if (data != null) {
String strData = new String(data, UTF_8);
if (!strData.equals("manager")) {
- servers.add(new ThriftTransportKey(
- new ServerServices(strData).getAddress(Service.TSERV_CLIENT), rpcTimeout, context));
- }
- }
- }
-
- boolean opened = false;
- try {
- Pair<String,TTransport> pair =
- context.getTransportPool().getAnyTransport(servers, preferCachedConnections);
- C client = ThriftUtil.createClient(type, pair.getSecond());
- opened = true;
- warned.set(false);
- return new Pair<>(pair.getFirst(), client);
- } finally {
- if (!opened) {
- if (warned.compareAndSet(false, true)) {
- if (servers.isEmpty()) {
- LOG.warn("There are no tablet servers: check that zookeeper and accumulo are running.");
- } else {
- LOG.warn("Failed to find an available server in the list of servers: {}", servers);
+ final HostAndPort tserverClientAddress =
+ new ServerServices(strData).getAddress(Service.TSERV_CLIENT);
+ try {
+ TTransport transport = context.getTransportPool().getTransport(type,
+ tserverClientAddress, rpcTimeout, context, preferCachedConnections);
+ C client = ThriftUtil.createClient(type, transport);
+ warned.set(false);
+ return new Pair<String,C>(tserverClientAddress.toString(), client);
+ } catch (TTransportException e) {
+ LOG.trace("Error creating transport to {}", tserverClientAddress);
+ continue;
}
}
}
}
+ LOG.warn("Failed to find an available server in the list of servers: {}", tservers);
+ throw new TTransportException("Failed to connect to any server");
Review Comment:
Added this in 1a3d5e0
--
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
Re: [PR] Optimized logic for getting a random TabletServer connection [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4309:
URL: https://github.com/apache/accumulo/pull/4309#discussion_r1506070055
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -57,44 +60,54 @@ default Pair<String,C> getTabletServerConnection(Logger LOG, ThriftClientTypes<C
ClientContext context, boolean preferCachedConnections, AtomicBoolean warned)
throws TTransportException {
checkArgument(context != null, "context is null");
- long rpcTimeout = context.getClientTimeoutInMillis();
- // create list of servers
- ArrayList<ThriftTransportKey> servers = new ArrayList<>();
+ final long rpcTimeout = context.getClientTimeoutInMillis();
- // add tservers
- ZooCache zc = context.getZooCache();
- for (String tserver : zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS)) {
- var zLocPath =
- ServiceLock.path(context.getZooKeeperRoot() + Constants.ZTSERVERS + "/" + tserver);
- byte[] data = zc.getLockData(zLocPath);
- if (data != null) {
- String strData = new String(data, UTF_8);
- if (!strData.equals("manager")) {
- servers.add(new ThriftTransportKey(
- new ServerServices(strData).getAddress(Service.TSERV_CLIENT), rpcTimeout, context));
+ final ZooCache zc = context.getZooCache();
+ final List<String> tservers = new ArrayList<>();
+ final AtomicBoolean warnedAboutTServersBeingDown = new AtomicBoolean(false);
+
+ for (int retries = 0; retries < 10; retries++) {
+ // Cluster may not be up, wait for tservers to come online
+ while (true) {
+ tservers.addAll(zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS));
+
+ if (!tservers.isEmpty()) {
+ break;
}
+
+ if (tservers.isEmpty() && !warnedAboutTServersBeingDown.get()) {
+ LOG.warn("There are no tablet servers: check that zookeeper and accumulo are running.");
+ warnedAboutTServersBeingDown.set(true);
+ }
+ UtilWaitThread.sleep(100);
}
- }
- boolean opened = false;
- try {
- Pair<String,TTransport> pair =
- context.getTransportPool().getAnyTransport(servers, preferCachedConnections);
- C client = ThriftUtil.createClient(type, pair.getSecond());
- opened = true;
- warned.set(false);
- return new Pair<>(pair.getFirst(), client);
- } finally {
- if (!opened) {
- if (warned.compareAndSet(false, true)) {
- if (servers.isEmpty()) {
- LOG.warn("There are no tablet servers: check that zookeeper and accumulo are running.");
- } else {
- LOG.warn("Failed to find an available server in the list of servers: {}", servers);
+ // Try to connect to an online tserver
+ Collections.shuffle(tservers);
+ for (String tserver : tservers) {
+ var zLocPath =
+ ServiceLock.path(context.getZooKeeperRoot() + Constants.ZTSERVERS + "/" + tserver);
+ byte[] data = zc.getLockData(zLocPath);
+ if (data != null) {
+ String strData = new String(data, UTF_8);
+ if (!strData.equals("manager")) {
Review Comment:
We probably need an issue for this then.
--
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
Re: [PR] Optimized logic for getting a random TabletServer connection [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4309:
URL: https://github.com/apache/accumulo/pull/4309#discussion_r1508937794
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -57,44 +59,56 @@ default Pair<String,C> getTabletServerConnection(Logger LOG, ThriftClientTypes<C
ClientContext context, boolean preferCachedConnections, AtomicBoolean warned)
throws TTransportException {
checkArgument(context != null, "context is null");
- long rpcTimeout = context.getClientTimeoutInMillis();
- // create list of servers
- ArrayList<ThriftTransportKey> servers = new ArrayList<>();
- // add tservers
- ZooCache zc = context.getZooCache();
- for (String tserver : zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS)) {
+ if (preferCachedConnections) {
+ Pair<String,TTransport> cachedTransport =
+ context.getTransportPool().getAnyCachedTransport(type);
+ if (cachedTransport != null) {
+ C client = ThriftUtil.createClient(type, cachedTransport.getSecond());
+ warned.set(false);
+ return new Pair<String,C>(cachedTransport.getFirst(), client);
+ }
+ }
+
+ final long rpcTimeout = context.getClientTimeoutInMillis();
+ final ZooCache zc = context.getZooCache();
+ final List<String> tservers = new ArrayList<>();
+
+ tservers.addAll(zc.getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS));
+
+ if (tservers.isEmpty()) {
+ if (warned.compareAndSet(false, true)) {
+ LOG.warn("There are no tablet servers: check that zookeeper and accumulo are running.");
Review Comment:
> Is this code used for more than tservers now?
No, it's only used by the ClientServiceThriftClient (the generic Client API) and the TabletServerThriftClient (for the TabletClientService).
--
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
Re: [PR] Optimized logic for getting a random TabletServer connection [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on PR #4309:
URL: https://github.com/apache/accumulo/pull/4309#issuecomment-1973789478
Full IT build based off commit aecd79064602b2dca413e4274e2c1ba98bbfc048 passed successfully.
--
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
Re: [PR] Optimized logic for getting a random TabletServer connection [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion merged PR #4309:
URL: https://github.com/apache/accumulo/pull/4309
--
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