keith-turner commented on code in PR #5256:
URL: https://github.com/apache/accumulo/pull/5256#discussion_r1934335279
##########
server/base/src/main/java/org/apache/accumulo/server/manager/LiveTServerSet.java:
##########
@@ -307,32 +307,59 @@ private synchronized void checkServer(final
Set<TServerInstance> updates,
@Override
public void accept(WatchedEvent event) {
+ log.trace("Received event: {}", event);
// its important that these event are propagated by ZooCache, because this
ensures when reading
// zoocache that is has already processed the event and cleared
// relevant nodes before code below reads from zoocache
-
- if (event.getPath() != null) {
- if (event.getPath().endsWith(Constants.ZTSERVERS)) {
+ final String tserverZPath = context.getZooKeeperRoot() +
Constants.ZTSERVERS;
+ if (event.getPath() != null && event.getPath().startsWith(tserverZPath)) {
+ if (event.getPath().equals(tserverZPath)) {
scanServers();
- } else {
- try {
- final ServiceLockPath slp =
- ServiceLockPaths.parse(Optional.of(Constants.ZTSERVERS),
event.getPath());
- if (slp.getType().equals(Constants.ZTSERVERS)) {
- try {
- final Set<TServerInstance> updates = new HashSet<>();
- final Set<TServerInstance> doomed = new HashSet<>();
- checkServer(updates, doomed, slp);
- this.cback.update(this, doomed, updates);
- } catch (Exception ex) {
- log.error("Error processing event for tserver: " +
slp.toString(), ex);
+ } else if (event.getPath().contains(Constants.ZTSERVERS)) {
+ // It's possible that the path contains more than the tserver address,
it
+ // could contain it's children. We need to fix the path before parsing
it
+ // path should be: zooKeeperRoot + Constants.ZTSERVERS + "/" +
resourceGroup + "/" address
+ String pathToUse = null;
+ String remaining = event.getPath().substring(tserverZPath.length() +
1);
+ int numSlashes = StringUtils.countMatches(remaining, '/');
+ if (numSlashes == 1) {
+ // event path is the server
+ pathToUse = event.getPath();
+ } else if (numSlashes > 1) {
+ // event path is the children of the server, maybe zlock
+ int idx = remaining.indexOf("/");
+ String rg = remaining.substring(0, idx);
+ String server = remaining.substring(idx + 1, remaining.indexOf("/",
idx + 1));
+ pathToUse = tserverZPath + "/" + rg + "/" + server;
+ } else {
+ // malformed path
+ pathToUse = null;
+ log.debug("Received event for path that can't be parsed, path: " +
event.getPath());
+ }
+ if (pathToUse != null) {
+ try {
+ final ServiceLockPath slp =
+ ServiceLockPaths.parse(Optional.of(Constants.ZTSERVERS),
pathToUse);
+ if (slp.getType().equals(Constants.ZTSERVERS)) {
+ try {
+ log.trace("Processing event for server: {}", slp.toString());
+ final Set<TServerInstance> updates = new HashSet<>();
+ final Set<TServerInstance> doomed = new HashSet<>();
+ checkServer(updates, doomed, slp);
+ this.cback.update(this, doomed, updates);
+ } catch (Exception ex) {
+ log.error("Error processing event for tserver: " +
slp.toString(), ex);
+ }
}
+ } catch (IllegalArgumentException e) {
+ log.debug("Received event for path that can't be parsed, path: " +
event.getPath());
Review Comment:
Would be useful to log the original and derived path that was passed to
parse.
```suggestion
log.debug("Received event for path that can't be parsed, path:
{} pathToUse:{} ", event.getPath(),pathToUse);
```
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
