You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "ddanielr (via GitHub)" <gi...@apache.org> on 2026/04/01 00:36:24 UTC
[PR] Adds AssistantManagerThriftRPCs [accumulo]
ddanielr opened a new pull request, #6284:
URL: https://github.com/apache/accumulo/pull/6284
Renames the current ManagerClientService to PrimaryManagerClientService.
Adds an AssistantManagerClientService thrift RPC to the manager.
Migrated flush and property actions to the AssistantManagerClientService.
relates to #6183
--
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] Adds AssistantManagerThriftRPCs [accumulo]
Posted by "ddanielr (via GitHub)" <gi...@apache.org>.
ddanielr commented on code in PR #6284:
URL: https://github.com/apache/accumulo/pull/6284#discussion_r3028474458
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/AssistantManagerThriftClient.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.rpc.clients;
+
+import static com.google.common.util.concurrent.Uninterruptibles.sleepUninterruptibly;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+
+import java.util.ConcurrentModificationException;
+
+import org.apache.accumulo.core.client.AccumuloException;
+import org.apache.accumulo.core.client.AccumuloSecurityException;
+import org.apache.accumulo.core.client.NamespaceNotFoundException;
+import org.apache.accumulo.core.client.TableNotFoundException;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftConcurrentModificationException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftNotActiveServiceException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftSecurityException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftTableOperationException;
+import org.apache.accumulo.core.manager.thrift.AssistantManagerClientService.Client;
+import org.apache.accumulo.core.rpc.ThriftUtil;
+import org.apache.thrift.transport.TTransportException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class AssistantManagerThriftClient extends ThriftClientTypes<Client>
+ implements ManagerClient<Client> {
+
+ private static final Logger LOG = LoggerFactory.getLogger(AssistantManagerThriftClient.class);
+
+ AssistantManagerThriftClient(String serviceName) {
+ super(serviceName, new Client.Factory());
+ }
+
+ @Override
+ public Client getConnection(ClientContext context) {
+ return getManagerConnection(LOG, this, context);
Review Comment:
@keith-turner I saw the code in FateManager, but the FateThriftClient still used the default getConnection method from ManagerClient.
Should `FateThriftClient` also be updated to select from the assistant Managers?
--
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] Adds AssistantManagerThriftRPCs [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #6284:
URL: https://github.com/apache/accumulo/pull/6284#discussion_r3027826401
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/AssistantManagerThriftClient.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.rpc.clients;
+
+import static com.google.common.util.concurrent.Uninterruptibles.sleepUninterruptibly;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+
+import java.util.ConcurrentModificationException;
+
+import org.apache.accumulo.core.client.AccumuloException;
+import org.apache.accumulo.core.client.AccumuloSecurityException;
+import org.apache.accumulo.core.client.NamespaceNotFoundException;
+import org.apache.accumulo.core.client.TableNotFoundException;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftConcurrentModificationException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftNotActiveServiceException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftSecurityException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftTableOperationException;
+import org.apache.accumulo.core.manager.thrift.AssistantManagerClientService.Client;
+import org.apache.accumulo.core.rpc.ThriftUtil;
+import org.apache.thrift.transport.TTransportException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class AssistantManagerThriftClient extends ThriftClientTypes<Client>
+ implements ManagerClient<Client> {
+
+ private static final Logger LOG = LoggerFactory.getLogger(AssistantManagerThriftClient.class);
+
+ AssistantManagerThriftClient(String serviceName) {
+ super(serviceName, new Client.Factory());
+ }
+
+ @Override
+ public Client getConnection(ClientContext context) {
+ return getManagerConnection(LOG, this, context);
Review Comment:
It would be nice if we could tie in TServerClient.getThriftServerConnection somehow. Then the debug property would apply to Manager connections as well.
--
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] Adds AssistantManagerThriftRPCs [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #6284:
URL: https://github.com/apache/accumulo/pull/6284#discussion_r3022883766
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/AssistantManagerThriftClient.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.rpc.clients;
+
+import static com.google.common.util.concurrent.Uninterruptibles.sleepUninterruptibly;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+
+import java.util.ConcurrentModificationException;
+
+import org.apache.accumulo.core.client.AccumuloException;
+import org.apache.accumulo.core.client.AccumuloSecurityException;
+import org.apache.accumulo.core.client.NamespaceNotFoundException;
+import org.apache.accumulo.core.client.TableNotFoundException;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftConcurrentModificationException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftNotActiveServiceException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftSecurityException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftTableOperationException;
+import org.apache.accumulo.core.manager.thrift.AssistantManagerClientService.Client;
+import org.apache.accumulo.core.rpc.ThriftUtil;
+import org.apache.thrift.transport.TTransportException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class AssistantManagerThriftClient extends ThriftClientTypes<Client>
+ implements ManagerClient<Client> {
+
+ private static final Logger LOG = LoggerFactory.getLogger(AssistantManagerThriftClient.class);
+
+ AssistantManagerThriftClient(String serviceName) {
+ super(serviceName, new Client.Factory());
+ }
+
+ @Override
+ public Client getConnection(ClientContext context) {
+ return getManagerConnection(LOG, this, context);
+ }
+
+ public <R> R executeTableCommand(ClientContext context, Exec<R,Client> exec)
Review Comment:
May need to create a new super type that the primary and assistant manager share, not sure. See that a fate client also extends the base type and it may not need some of these methods.
--
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] Adds AssistantManagerThriftRPCs [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #6284:
URL: https://github.com/apache/accumulo/pull/6284#discussion_r3053383771
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/AssistantManagerThriftClient.java:
##########
@@ -32,24 +41,109 @@
import org.apache.accumulo.core.clientImpl.thrift.ThriftNotActiveServiceException;
import org.apache.accumulo.core.clientImpl.thrift.ThriftSecurityException;
import org.apache.accumulo.core.clientImpl.thrift.ThriftTableOperationException;
+import org.apache.accumulo.core.lock.ServiceLockData;
+import org.apache.accumulo.core.lock.ServiceLockData.ThriftService;
+import org.apache.accumulo.core.lock.ServiceLockPaths.AddressSelector;
+import org.apache.accumulo.core.lock.ServiceLockPaths.ServiceLockPath;
+import org.apache.accumulo.core.manager.thrift.AssistantManagerClientService;
import org.apache.accumulo.core.manager.thrift.AssistantManagerClientService.Client;
import org.apache.accumulo.core.rpc.ThriftUtil;
+import org.apache.accumulo.core.util.Pair;
+import org.apache.accumulo.core.zookeeper.ZooCache;
+import org.apache.thrift.transport.TTransport;
import org.apache.thrift.transport.TTransportException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.net.HostAndPort;
+
public class AssistantManagerThriftClient extends ThriftClientTypes<Client>
- implements ManagerClient<Client> {
+ implements TServerClient<Client> {
private static final Logger LOG = LoggerFactory.getLogger(AssistantManagerThriftClient.class);
+ private final AtomicBoolean warnedAboutAssistantManagersBeingDown = new AtomicBoolean(false);
AssistantManagerThriftClient(String serviceName) {
super(serviceName, new Client.Factory());
}
@Override
public Client getConnection(ClientContext context) {
- return getManagerConnection(LOG, this, context);
+ return getThriftServerConnection(context, false).getSecond();
+ }
+
+ @Override
+ public Pair<String,Client> getThriftServerConnection(ClientContext context,
+ boolean preferCachedConnections) {
+ try {
+ return getThriftServerConnection(LOG, this, context, preferCachedConnections,
+ warnedAboutAssistantManagersBeingDown, ThriftService.FATE_WORKER);
+ } catch (TTransportException tte) {
+ Throwable cause = tte.getCause();
+ if (cause instanceof UnknownHostException) {
+ // do not expect to recover from this
+ throw new IllegalStateException(tte);
+ }
+ LOG.debug("Failed to connect to a assistant manager, will retry... ", tte);
+ return null;
+ }
+ }
+
+ public Pair<String,Client> getThriftServerConnection(Logger LOG, ThriftClientTypes<Client> type,
+ ClientContext context, boolean preferCachedConnections, AtomicBoolean warned,
+ ThriftService service) throws TTransportException {
+ checkArgument(context != null, "context is null");
+ final long rpcTimeout = context.getClientTimeoutInMillis();
+ final ZooCache zc = context.getZooCache();
+ final String debugHost = System.getProperty(DEBUG_HOST, null);
Review Comment:
This does not seems to use the debugHost. Also not sure debugHost should be used here as it may be intended for the general client service that runs on all servers? Not sure though. If I set the debug host to grab a site config from a specific tserver, then that client could never make a manager connection because the tservers and assitant managers are disjoint sets.
--
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] Adds AssistantManagerThriftRPCs [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #6284:
URL: https://github.com/apache/accumulo/pull/6284#discussion_r3022837047
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/AssistantManagerThriftClient.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.rpc.clients;
+
+import static com.google.common.util.concurrent.Uninterruptibles.sleepUninterruptibly;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+
+import java.util.ConcurrentModificationException;
+
+import org.apache.accumulo.core.client.AccumuloException;
+import org.apache.accumulo.core.client.AccumuloSecurityException;
+import org.apache.accumulo.core.client.NamespaceNotFoundException;
+import org.apache.accumulo.core.client.TableNotFoundException;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftConcurrentModificationException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftNotActiveServiceException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftSecurityException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftTableOperationException;
+import org.apache.accumulo.core.manager.thrift.AssistantManagerClientService.Client;
+import org.apache.accumulo.core.rpc.ThriftUtil;
+import org.apache.thrift.transport.TTransportException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class AssistantManagerThriftClient extends ThriftClientTypes<Client>
+ implements ManagerClient<Client> {
+
+ private static final Logger LOG = LoggerFactory.getLogger(AssistantManagerThriftClient.class);
+
+ AssistantManagerThriftClient(String serviceName) {
+ super(serviceName, new Client.Factory());
+ }
+
+ @Override
+ public Client getConnection(ClientContext context) {
+ return getManagerConnection(LOG, this, context);
Review Comment:
Seems like this will always connect to the primary manager. Need to call [this code] (https://github.com/apache/accumulo/blob/97b5c810f0cb14e43e3f51c5f83282dc59ecd1e3/server/manager/src/main/java/org/apache/accumulo/manager/fate/FateManager.java#L420) to get a list of assistant managers and pick one.
For picking one I have been considering using hashing for #6279 to better spread the connections. If we randomly pick one and we C clients then every assistant manager can have up to C connections or more. If we hash clients to AM assistant managers then each one will have on the order of C/AM connections. This could also help w/ debugging where a client goes to the same manager consistently. However not sure what to hash on in the client. For #6279 was thinking of doing hash mod using the compactor address. To start with it would probably be best to just randomly pick one though.
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/AssistantManagerThriftClient.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.rpc.clients;
+
+import static com.google.common.util.concurrent.Uninterruptibles.sleepUninterruptibly;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+
+import java.util.ConcurrentModificationException;
+
+import org.apache.accumulo.core.client.AccumuloException;
+import org.apache.accumulo.core.client.AccumuloSecurityException;
+import org.apache.accumulo.core.client.NamespaceNotFoundException;
+import org.apache.accumulo.core.client.TableNotFoundException;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftConcurrentModificationException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftNotActiveServiceException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftSecurityException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftTableOperationException;
+import org.apache.accumulo.core.manager.thrift.AssistantManagerClientService.Client;
+import org.apache.accumulo.core.rpc.ThriftUtil;
+import org.apache.thrift.transport.TTransportException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class AssistantManagerThriftClient extends ThriftClientTypes<Client>
+ implements ManagerClient<Client> {
+
+ private static final Logger LOG = LoggerFactory.getLogger(AssistantManagerThriftClient.class);
+
+ AssistantManagerThriftClient(String serviceName) {
+ super(serviceName, new Client.Factory());
+ }
+
+ @Override
+ public Client getConnection(ClientContext context) {
+ return getManagerConnection(LOG, this, context);
+ }
+
+ public <R> R executeTableCommand(ClientContext context, Exec<R,Client> exec)
Review Comment:
Was this code copied from MangerThriftClient? If so would be good to push to the super type and refactor for any specialization needed.
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/ManagerThriftClient.java:
##########
@@ -32,7 +32,7 @@
import org.apache.accumulo.core.clientImpl.thrift.ThriftNotActiveServiceException;
import org.apache.accumulo.core.clientImpl.thrift.ThriftSecurityException;
import org.apache.accumulo.core.clientImpl.thrift.ThriftTableOperationException;
-import org.apache.accumulo.core.manager.thrift.ManagerClientService.Client;
+import org.apache.accumulo.core.manager.thrift.PrimaryManagerClientService.Client;
Review Comment:
would be good to rename this class from ManagerThriftClient to PrimaryManagerThriftClient
--
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] Adds AssistantManagerThriftRPCs [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #6284:
URL: https://github.com/apache/accumulo/pull/6284#discussion_r3027878567
##########
server/manager/src/main/java/org/apache/accumulo/manager/Manager.java:
##########
Review Comment:
getAssistantManagerLock() should also change to advertise the new services the AssistantManager is servicing. This would allow clients to get the addresses for all managers, then select one that is advertising the service that the client needs. `TServerClient.getThriftServerConnection` does this right now, but only for a subset of services. The logic in that class could be expanded to handle more types.
--
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] Adds AssistantManagerThriftRPCs [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #6284:
URL: https://github.com/apache/accumulo/pull/6284#discussion_r3028542993
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/AssistantManagerThriftClient.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.rpc.clients;
+
+import static com.google.common.util.concurrent.Uninterruptibles.sleepUninterruptibly;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+
+import java.util.ConcurrentModificationException;
+
+import org.apache.accumulo.core.client.AccumuloException;
+import org.apache.accumulo.core.client.AccumuloSecurityException;
+import org.apache.accumulo.core.client.NamespaceNotFoundException;
+import org.apache.accumulo.core.client.TableNotFoundException;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftConcurrentModificationException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftNotActiveServiceException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftSecurityException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftTableOperationException;
+import org.apache.accumulo.core.manager.thrift.AssistantManagerClientService.Client;
+import org.apache.accumulo.core.rpc.ThriftUtil;
+import org.apache.thrift.transport.TTransportException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class AssistantManagerThriftClient extends ThriftClientTypes<Client>
+ implements ManagerClient<Client> {
+
+ private static final Logger LOG = LoggerFactory.getLogger(AssistantManagerThriftClient.class);
+
+ AssistantManagerThriftClient(String serviceName) {
+ super(serviceName, new Client.Factory());
+ }
+
+ @Override
+ public Client getConnection(ClientContext context) {
+ return getManagerConnection(LOG, this, context);
Review Comment:
> Should FateThriftClient also be updated to select from the assistant Managers?
Can not do that now, fate meta client is only available on the primary manager. After #6224,#6227, and #6232 are merged then both fate clients (meta and user) would be available on all managers. After those are merged could refactor the client code that initiates fate table ops to use any manger.
>@keith-turner I saw the code in FateManager, but the FateThriftClient still used the default getConnection method from ManagerClient.
The code in FateThriftClient is for clients that interact with creating and waiting on fate operations. The code in FateManager is distributing execution of fate operations across managers. So what we currently have is fate operations can execute on all managers, but accumulo clients must still work w/ the primary manager to initiate and wait on fate operations. After #6232 is merged then accumulo clients could interact w/ any manager.
--
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] Adds AssistantManagerThriftRPCs [accumulo]
Posted by "ddanielr (via GitHub)" <gi...@apache.org>.
ddanielr commented on code in PR #6284:
URL: https://github.com/apache/accumulo/pull/6284#discussion_r3029125691
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/AssistantManagerThriftClient.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.rpc.clients;
+
+import static com.google.common.util.concurrent.Uninterruptibles.sleepUninterruptibly;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+
+import java.util.ConcurrentModificationException;
+
+import org.apache.accumulo.core.client.AccumuloException;
+import org.apache.accumulo.core.client.AccumuloSecurityException;
+import org.apache.accumulo.core.client.NamespaceNotFoundException;
+import org.apache.accumulo.core.client.TableNotFoundException;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftConcurrentModificationException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftNotActiveServiceException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftSecurityException;
+import org.apache.accumulo.core.clientImpl.thrift.ThriftTableOperationException;
+import org.apache.accumulo.core.manager.thrift.AssistantManagerClientService.Client;
+import org.apache.accumulo.core.rpc.ThriftUtil;
+import org.apache.thrift.transport.TTransportException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class AssistantManagerThriftClient extends ThriftClientTypes<Client>
+ implements ManagerClient<Client> {
+
+ private static final Logger LOG = LoggerFactory.getLogger(AssistantManagerThriftClient.class);
+
+ AssistantManagerThriftClient(String serviceName) {
+ super(serviceName, new Client.Factory());
+ }
+
+ @Override
+ public Client getConnection(ClientContext context) {
+ return getManagerConnection(LOG, this, context);
Review Comment:
okay! I opened #6291 for simplifying the thrift client code a bit.
--
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