keith-turner commented on code in PR #5775:
URL: https://github.com/apache/accumulo/pull/5775#discussion_r2252318352
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -60,16 +63,45 @@
public interface TServerClient<C extends TServiceClient> {
static final String DEBUG_HOST = "org.apache.accumulo.client.rpc.debug.host";
+ static final String DEBUG_RG = "org.apache.accumulo.client.rpc.debug.group";
- Pair<String,C> getThriftServerConnection(ClientContext context, boolean
preferCachedConnections)
- throws TTransportException;
+ Pair<String,C> getThriftServerConnection(ClientContext context, boolean
preferCachedConnections,
+ ResourceGroupPredicate rgp) throws TTransportException;
default Pair<String,C> getThriftServerConnection(Logger LOG,
ThriftClientTypes<C> type,
ClientContext context, boolean preferCachedConnections, AtomicBoolean
warned,
- ThriftService service) throws TTransportException {
+ ThriftService service, ResourceGroupPredicate rgp) throws
TTransportException {
checkArgument(context != null, "context is null");
final String debugHost = System.getProperty(DEBUG_HOST, null);
+ final String debugRG = System.getProperty(DEBUG_RG, null);
+
+ if (debugHost != null && debugRG != null) {
+ LOG.warn("System properties {} and {} are both set. If set incorrectly
then"
+ + " this client may not find a server to connect to.", DEBUG_HOST,
DEBUG_RG);
+ }
+
+ if (debugRG != null) {
+ if (type == ThriftClientTypes.CLIENT || type ==
ThriftClientTypes.COMPACTOR
+ || type == ThriftClientTypes.SERVER_PROCESS || type ==
ThriftClientTypes.TABLET_INGEST
+ || type == ThriftClientTypes.TABLET_MGMT || type ==
ThriftClientTypes.TABLET_SCAN
+ || type == ThriftClientTypes.TABLET_SERVER) {
+ final ResourceGroupId dgid = ResourceGroupId.of(debugRG);
+ if (rgp.test(dgid)) {
+ // its safe to potentially narrow the predicate
+ LOG.debug("System property '{}' set to '{}' overriding predicate
argument", DEBUG_RG,
+ debugRG);
+ rgp = r -> r.equals(dgid);
+ } else {
+ LOG.warn("System property '{}' set to '{}' does not intersect with
predicate argument."
+ + " Ignoring degug system property.", DEBUG_RG, debugRG);
+ }
+ } else {
+ LOG.debug(
+ "System property '{}' set to '{}' but ignored when making RPCs to
management servers",
+ DEBUG_RG, debugRG);
+ }
+ }
Review Comment:
That code is already doing a linear scan through all of the cached
connections. If it knew the RG as part of the ThriftTransportKey then the
predicate could be passed down and used in the loop over the connections.
##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -60,16 +63,45 @@
public interface TServerClient<C extends TServiceClient> {
static final String DEBUG_HOST = "org.apache.accumulo.client.rpc.debug.host";
+ static final String DEBUG_RG = "org.apache.accumulo.client.rpc.debug.group";
- Pair<String,C> getThriftServerConnection(ClientContext context, boolean
preferCachedConnections)
- throws TTransportException;
+ Pair<String,C> getThriftServerConnection(ClientContext context, boolean
preferCachedConnections,
+ ResourceGroupPredicate rgp) throws TTransportException;
default Pair<String,C> getThriftServerConnection(Logger LOG,
ThriftClientTypes<C> type,
ClientContext context, boolean preferCachedConnections, AtomicBoolean
warned,
- ThriftService service) throws TTransportException {
+ ThriftService service, ResourceGroupPredicate rgp) throws
TTransportException {
checkArgument(context != null, "context is null");
final String debugHost = System.getProperty(DEBUG_HOST, null);
+ final String debugRG = System.getProperty(DEBUG_RG, null);
+
+ if (debugHost != null && debugRG != null) {
+ LOG.warn("System properties {} and {} are both set. If set incorrectly
then"
+ + " this client may not find a server to connect to.", DEBUG_HOST,
DEBUG_RG);
+ }
+
+ if (debugRG != null) {
+ if (type == ThriftClientTypes.CLIENT || type ==
ThriftClientTypes.COMPACTOR
+ || type == ThriftClientTypes.SERVER_PROCESS || type ==
ThriftClientTypes.TABLET_INGEST
+ || type == ThriftClientTypes.TABLET_MGMT || type ==
ThriftClientTypes.TABLET_SCAN
+ || type == ThriftClientTypes.TABLET_SERVER) {
+ final ResourceGroupId dgid = ResourceGroupId.of(debugRG);
+ if (rgp.test(dgid)) {
+ // its safe to potentially narrow the predicate
+ LOG.debug("System property '{}' set to '{}' overriding predicate
argument", DEBUG_RG,
+ debugRG);
+ rgp = r -> r.equals(dgid);
+ } else {
+ LOG.warn("System property '{}' set to '{}' does not intersect with
predicate argument."
+ + " Ignoring degug system property.", DEBUG_RG, debugRG);
+ }
+ } else {
+ LOG.debug(
+ "System property '{}' set to '{}' but ignored when making RPCs to
management servers",
+ DEBUG_RG, debugRG);
+ }
+ }
Review Comment:
https://github.com/apache/accumulo/blob/0863c9e12498930e7052b6eff4cef246c05b9780/core/src/main/java/org/apache/accumulo/core/clientImpl/ThriftTransportPool.java#L142
--
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]
