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/10/08 17:03:58 UTC
[PR] Expand types of servers that respond to ping command [accumulo]
dlmarion opened a new pull request, #4958:
URL: https://github.com/apache/accumulo/pull/4958
Closes #4952
--
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] Expand types of servers that respond to ping command [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4958:
URL: https://github.com/apache/accumulo/pull/4958#discussion_r1793994098
##########
core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java:
##########
@@ -308,10 +308,9 @@ List<ActiveCompaction> getActiveCompactions(ServerId server)
List<ActiveCompaction> getActiveCompactions() throws AccumuloException, AccumuloSecurityException;
/**
- * Throws an exception if a tablet server can not be contacted.
+ * Throws an exception if a compactor, scan server, or tablet server can not be contacted.
*
- * @param tserver The tablet server address. This should be of the form
- * {@code <ip address>:<port>}
+ * @param tserver The server address. This should be of the form {@code <ip address>:<port>}
Review Comment:
I'm thinking we might want to leave this as-is. Reason being that if a user constructs a ServerId as an argument to this method, then they might expect that this method will use the type or resource group members as part of the implementation. It doesn't it just uses the `host:port`. If the user already has a ServerId, then they can pass `ServerId.toHostPortString` when calling 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] Expand types of servers that respond to ping command [accumulo]
Posted by "ddanielr (via GitHub)" <gi...@apache.org>.
ddanielr commented on code in PR #4958:
URL: https://github.com/apache/accumulo/pull/4958#discussion_r1792538209
##########
shell/src/main/java/org/apache/accumulo/shell/commands/PingCommand.java:
##########
@@ -31,31 +31,37 @@
public class PingCommand extends Command {
- private Option tserverOption, disablePaginationOpt;
+ private Option serverOption, tserverOption, disablePaginationOpt;
Review Comment:
Tested this with uno. The `-ts` flag option has the same usage as `-s`
```
root@uno> ping --help
usage: ping [-?] [-np] [-s <server address>] [-ts <tablet server>]
description: ping compactors, scan servers, or tablet servers
-?,--help display this help
-np,--no-pagination disable pagination of output
-s,--server <server address> compactor, scan server, or tablet server address to ping
-ts,--tabletServer <tablet server> compactor, scan server, or tablet server address to ping
```
I'm able to pass a compactor `host:port` string with either the `-ts` or `-s` option flags and they both work.
```
[lock.ServiceLock] DEBUG: [zlock#1aa7a951-9a79-498c-8f74-69cf3c6f4469#] Ephemeral node /accumulo/e4262575-b573-48cc-b4ae-54c2284dff33/compactors/default/localhost:9133/zlock#1aa7a951-9a79-498c-8f74-69cf3c6f4469#0000000000 created with data: {"descriptors":[{"uuid":"1aa7a951-9a79-498c-8f74-69cf3c6f4469","service":"CLIENT","address":"localhost:9133","group":"default"},{"uuid":"1aa7a951-9a79-498c-8f74-69cf3c6f4469","service":"COMPACTOR","address":"localhost:9133","group":"default"}]}
root@uno> ping -ts localhost:9133
localhost:9133 OK
root@uno> ping -s localhost:9133
localhost:9133 OK
```
Given that, does the `-ts` flag still need to exist or can it just be replaced with `-s`?
--
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] Expand types of servers that respond to ping command [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion merged PR #4958:
URL: https://github.com/apache/accumulo/pull/4958
--
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] Expand types of servers that respond to ping command [accumulo]
Posted by "ddanielr (via GitHub)" <gi...@apache.org>.
ddanielr commented on code in PR #4958:
URL: https://github.com/apache/accumulo/pull/4958#discussion_r1792538209
##########
shell/src/main/java/org/apache/accumulo/shell/commands/PingCommand.java:
##########
@@ -31,31 +31,37 @@
public class PingCommand extends Command {
- private Option tserverOption, disablePaginationOpt;
+ private Option serverOption, tserverOption, disablePaginationOpt;
Review Comment:
Tested this with uno. The `-ts` flag option has the same usage as `-s`
```
root@uno> ping --help
usage: ping [-?] [-np] [-s <server address>] [-ts <tablet server>]
description: ping compactors, scan servers, or tablet servers
-?,--help display this help
-np,--no-pagination disable pagination of output
-s,--server <server address> compactor, scan server, or tablet server address to ping
-ts,--tabletServer <tablet server> compactor, scan server, or tablet server address to ping
```
I'm able to pass a compactor `host:port` string with either the `-ts` or `-s` option flags and they both work.
```
[lock.ServiceLock] DEBUG: [zlock#1aa7a951-9a79-498c-8f74-69cf3c6f4469#] Ephemeral node /accumulo/e4262575-b573-48cc-b4ae-54c2284dff33/compactors/default/localhost:9133/zlock#1aa7a951-9a79-498c-8f74-69cf3c6f4469#0000000000 created with data: {"descriptors":[{"uuid":"1aa7a951-9a79-498c-8f74-69cf3c6f4469","service":"CLIENT","address":"localhost:9133","group":"default"},{"uuid":"1aa7a951-9a79-498c-8f74-69cf3c6f4469","service":"COMPACTOR","address":"localhost:9133","group":"default"}]}
root@uno> ping -ts localhost:9133
localhost:9133 OK
root@uno> ping -s localhost:9133
localhost:9133 OK
```
Given that, does the `-ts` flag still need to exist or can it just be replaced with `-s`?
--
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] Expand types of servers that respond to ping command [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4958:
URL: https://github.com/apache/accumulo/pull/4958#discussion_r1792510836
##########
core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java:
##########
@@ -308,10 +308,9 @@ List<ActiveCompaction> getActiveCompactions(ServerId server)
List<ActiveCompaction> getActiveCompactions() throws AccumuloException, AccumuloSecurityException;
/**
- * Throws an exception if a tablet server can not be contacted.
+ * Throws an exception if a compactor, scan server, or tablet server can not be contacted.
*
- * @param tserver The tablet server address. This should be of the form
- * {@code <ip address>:<port>}
+ * @param tserver The server address. This should be of the form {@code <ip address>:<port>}
Review Comment:
Added #4964 for deprecating the old method
--
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] Expand types of servers that respond to ping command [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4958:
URL: https://github.com/apache/accumulo/pull/4958#discussion_r1792510836
##########
core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java:
##########
@@ -308,10 +308,9 @@ List<ActiveCompaction> getActiveCompactions(ServerId server)
List<ActiveCompaction> getActiveCompactions() throws AccumuloException, AccumuloSecurityException;
/**
- * Throws an exception if a tablet server can not be contacted.
+ * Throws an exception if a compactor, scan server, or tablet server can not be contacted.
*
- * @param tserver The tablet server address. This should be of the form
- * {@code <ip address>:<port>}
+ * @param tserver The server address. This should be of the form {@code <ip address>:<port>}
Review Comment:
Added #4964 for 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] Expand types of servers that respond to ping command [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #4958:
URL: https://github.com/apache/accumulo/pull/4958#discussion_r1792441771
##########
core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java:
##########
@@ -308,10 +308,9 @@ List<ActiveCompaction> getActiveCompactions(ServerId server)
List<ActiveCompaction> getActiveCompactions() throws AccumuloException, AccumuloSecurityException;
/**
- * Throws an exception if a tablet server can not be contacted.
+ * Throws an exception if a compactor, scan server, or tablet server can not be contacted.
*
- * @param tserver The tablet server address. This should be of the form
- * {@code <ip address>:<port>}
+ * @param tserver The server address. This should be of the form {@code <ip address>:<port>}
Review Comment:
Could deprecate this API and add a new `ping(ServerId)` method.
--
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] Expand types of servers that respond to ping command [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4958:
URL: https://github.com/apache/accumulo/pull/4958#discussion_r1793376956
##########
core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java:
##########
@@ -308,10 +308,9 @@ List<ActiveCompaction> getActiveCompactions(ServerId server)
List<ActiveCompaction> getActiveCompactions() throws AccumuloException, AccumuloSecurityException;
/**
- * Throws an exception if a tablet server can not be contacted.
+ * Throws an exception if a compactor, scan server, or tablet server can not be contacted.
*
- * @param tserver The tablet server address. This should be of the form
- * {@code <ip address>:<port>}
+ * @param tserver The server address. This should be of the form {@code <ip address>:<port>}
Review Comment:
See comment at https://github.com/apache/accumulo/pull/4964#issuecomment-2402099388. I'm thinking we might want to leave this as-is.
--
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] Expand types of servers that respond to ping command [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4958:
URL: https://github.com/apache/accumulo/pull/4958#discussion_r1793980028
##########
core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java:
##########
@@ -308,10 +308,9 @@ List<ActiveCompaction> getActiveCompactions(ServerId server)
List<ActiveCompaction> getActiveCompactions() throws AccumuloException, AccumuloSecurityException;
/**
- * Throws an exception if a tablet server can not be contacted.
+ * Throws an exception if a compactor, scan server, or tablet server can not be contacted.
*
- * @param tserver The tablet server address. This should be of the form
- * {@code <ip address>:<port>}
+ * @param tserver The server address. This should be of the form {@code <ip address>:<port>}
Review Comment:
Closed #4964
--
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] Expand types of servers that respond to ping command [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4958:
URL: https://github.com/apache/accumulo/pull/4958#discussion_r1794063224
##########
core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java:
##########
@@ -308,10 +308,9 @@ List<ActiveCompaction> getActiveCompactions(ServerId server)
List<ActiveCompaction> getActiveCompactions() throws AccumuloException, AccumuloSecurityException;
/**
- * Throws an exception if a tablet server can not be contacted.
+ * Throws an exception if a compactor, scan server, or tablet server can not be contacted.
*
- * @param tserver The tablet server address. This should be of the form
- * {@code <ip address>:<port>}
+ * @param tserver The server address. This should be of the form {@code <ip address>:<port>}
Review Comment:
Decided to add `ping(ServerId)` in addition to `ping(String)`, in 4c9f311
--
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