Skip to content

Commit

Permalink
ZOOKEEPER-4750: Align RequestPathMetricsCollector with FinalRequestPr…
Browse files Browse the repository at this point in the history 
…ocessor and OpCode

Currently, some path aware opcodes were not reported in `FinalRequestProcessor`
and some were not handled in `RequestPathMetricsCollector`. This commit aligns
them with `ZooDefs.OpCode`.
  • Loading branch information
@kezhuw
kezhuw committed May 22, 2024
1 parent 66202cb commit b18b7bd
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 2 deletions.
4 changes: 4 additions & 0 deletions zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ public void processRequest(Request request) {
((QuorumZooKeeperServer) zks).self.getQuorumVerifier().toString().getBytes(UTF_8),
rc.stat);
err = Code.get(rc.err);
requestPathMetricsCollector.registerRequest(request.type, rc.path);
break;
}
case OpCode.setACL: {
Expand Down Expand Up @@ -424,6 +425,7 @@ public void processRequest(Request request) {
AddWatchRequest addWatcherRequest = request.readRequestRecord(AddWatchRequest::new);
zks.getZKDatabase().addWatch(addWatcherRequest.getPath(), cnxn, addWatcherRequest.getMode());
rsp = new ErrorResponse(0);
requestPathMetricsCollector.registerRequest(request.type, addWatcherRequest.getPath());
break;
}
case OpCode.getACL: {
Expand Down Expand Up @@ -493,6 +495,7 @@ public void processRequest(Request request) {
null);
int number = zks.getZKDatabase().getAllChildrenNumber(path);
rsp = new GetAllChildrenNumberResponse(number);
requestPathMetricsCollector.registerRequest(request.type, path);
break;
}
case OpCode.getChildren2: {
Expand Down Expand Up @@ -563,6 +566,7 @@ public void processRequest(Request request) {
}
}
rsp = new GetEphemeralsResponse(ephemerals);
requestPathMetricsCollector.registerRequest(request.type, prefixPath);
break;
}
}
Expand Down
15 changes: 13 additions & 2 deletions ...er-server/src/main/java/org/apache/zookeeper/server/util/RequestPathMetricsCollector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,26 @@

package org.apache.zookeeper.server.util;

import static org.apache.zookeeper.ZooDefs.OpCode.addWatch;
import static org.apache.zookeeper.ZooDefs.OpCode.check;
import static org.apache.zookeeper.ZooDefs.OpCode.checkWatches;
import static org.apache.zookeeper.ZooDefs.OpCode.create;
import static org.apache.zookeeper.ZooDefs.OpCode.create2;
import static org.apache.zookeeper.ZooDefs.OpCode.createContainer;
import static org.apache.zookeeper.ZooDefs.OpCode.createTTL;
import static org.apache.zookeeper.ZooDefs.OpCode.delete;
import static org.apache.zookeeper.ZooDefs.OpCode.deleteContainer;
import static org.apache.zookeeper.ZooDefs.OpCode.exists;
import static org.apache.zookeeper.ZooDefs.OpCode.getACL;
import static org.apache.zookeeper.ZooDefs.OpCode.getAllChildrenNumber;
import static org.apache.zookeeper.ZooDefs.OpCode.getChildren;
import static org.apache.zookeeper.ZooDefs.OpCode.getChildren2;
import static org.apache.zookeeper.ZooDefs.OpCode.getData;
import static org.apache.zookeeper.ZooDefs.OpCode.getEphemerals;
import static org.apache.zookeeper.ZooDefs.OpCode.reconfig;
import static org.apache.zookeeper.ZooDefs.OpCode.removeWatches;
import static org.apache.zookeeper.ZooDefs.OpCode.setACL;
import static org.apache.zookeeper.ZooDefs.OpCode.setData;
import static org.apache.zookeeper.ZooDefs.OpCode.setWatches2;
import static org.apache.zookeeper.ZooDefs.OpCode.sync;
import java.io.PrintWriter;
import java.util.Arrays;
Expand Down Expand Up @@ -118,6 +123,7 @@ public RequestPathMetricsCollector(boolean accurateMode) {
scheduledExecutor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
requestsMap.put(Request.op2String(create), new PathStatsQueue(create));
requestsMap.put(Request.op2String(create2), new PathStatsQueue(create2));
requestsMap.put(Request.op2String(createTTL), new PathStatsQueue(createTTL));
requestsMap.put(Request.op2String(createContainer), new PathStatsQueue(createContainer));
requestsMap.put(Request.op2String(deleteContainer), new PathStatsQueue(deleteContainer));
requestsMap.put(Request.op2String(delete), new PathStatsQueue(delete));
Expand All @@ -128,10 +134,14 @@ public RequestPathMetricsCollector(boolean accurateMode) {
requestsMap.put(Request.op2String(setACL), new PathStatsQueue(setACL));
requestsMap.put(Request.op2String(getChildren), new PathStatsQueue(getChildren));
requestsMap.put(Request.op2String(getChildren2), new PathStatsQueue(getChildren2));
requestsMap.put(Request.op2String(getAllChildrenNumber), new PathStatsQueue(getAllChildrenNumber));
requestsMap.put(Request.op2String(checkWatches), new PathStatsQueue(checkWatches));
requestsMap.put(Request.op2String(removeWatches), new PathStatsQueue(removeWatches));
requestsMap.put(Request.op2String(setWatches2), new PathStatsQueue(setWatches2));
requestsMap.put(Request.op2String(addWatch), new PathStatsQueue(addWatch));
requestsMap.put(Request.op2String(sync), new PathStatsQueue(sync));
requestsMap.put(Request.op2String(check), new PathStatsQueue(check));
requestsMap.put(Request.op2String(getEphemerals), new PathStatsQueue(getEphemerals));
requestsMap.put(Request.op2String(reconfig), new PathStatsQueue(reconfig));
this.immutableRequestsMap = java.util.Collections.unmodifiableMap(requestsMap);
}

Expand All @@ -140,6 +150,7 @@ static boolean isWriteOp(int requestType) {
case ZooDefs.OpCode.sync:
case ZooDefs.OpCode.create:
case ZooDefs.OpCode.create2:
case ZooDefs.OpCode.createTTL:
case ZooDefs.OpCode.createContainer:
case ZooDefs.OpCode.delete:
case ZooDefs.OpCode.deleteContainer:
Expand Down
125 changes: 125 additions & 0 deletions ...erver/src/test/java/org/apache/zookeeper/server/util/RequestPathMetricsCollectorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,57 @@

package org.apache.zookeeper.server.util;

import static org.apache.zookeeper.ZooDefs.OpCode.addWatch;
import static org.apache.zookeeper.ZooDefs.OpCode.auth;
import static org.apache.zookeeper.ZooDefs.OpCode.check;
import static org.apache.zookeeper.ZooDefs.OpCode.checkWatches;
import static org.apache.zookeeper.ZooDefs.OpCode.closeSession;
import static org.apache.zookeeper.ZooDefs.OpCode.create;
import static org.apache.zookeeper.ZooDefs.OpCode.create2;
import static org.apache.zookeeper.ZooDefs.OpCode.createContainer;
import static org.apache.zookeeper.ZooDefs.OpCode.createSession;
import static org.apache.zookeeper.ZooDefs.OpCode.createTTL;
import static org.apache.zookeeper.ZooDefs.OpCode.delete;
import static org.apache.zookeeper.ZooDefs.OpCode.deleteContainer;
import static org.apache.zookeeper.ZooDefs.OpCode.error;
import static org.apache.zookeeper.ZooDefs.OpCode.exists;
import static org.apache.zookeeper.ZooDefs.OpCode.getACL;
import static org.apache.zookeeper.ZooDefs.OpCode.getAllChildrenNumber;
import static org.apache.zookeeper.ZooDefs.OpCode.getChildren;
import static org.apache.zookeeper.ZooDefs.OpCode.getChildren2;
import static org.apache.zookeeper.ZooDefs.OpCode.getData;
import static org.apache.zookeeper.ZooDefs.OpCode.getEphemerals;
import static org.apache.zookeeper.ZooDefs.OpCode.multi;
import static org.apache.zookeeper.ZooDefs.OpCode.multiRead;
import static org.apache.zookeeper.ZooDefs.OpCode.notification;
import static org.apache.zookeeper.ZooDefs.OpCode.ping;
import static org.apache.zookeeper.ZooDefs.OpCode.reconfig;
import static org.apache.zookeeper.ZooDefs.OpCode.removeWatches;
import static org.apache.zookeeper.ZooDefs.OpCode.sasl;
import static org.apache.zookeeper.ZooDefs.OpCode.setACL;
import static org.apache.zookeeper.ZooDefs.OpCode.setData;
import static org.apache.zookeeper.ZooDefs.OpCode.setWatches;
import static org.apache.zookeeper.ZooDefs.OpCode.setWatches2;
import static org.apache.zookeeper.ZooDefs.OpCode.sync;
import static org.apache.zookeeper.ZooDefs.OpCode.whoAmI;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.lang.reflect.Field;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.Random;
import java.util.concurrent.Executors;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.stream.IntStream;
import org.apache.zookeeper.ZooDefs;
import org.apache.zookeeper.server.Request;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
Expand Down Expand Up @@ -448,4 +484,93 @@ public void testMultiThreadPerf() throws InterruptedException {
assertTrue(TimeUnit.MILLISECONDS.toSeconds(endTime - startTime) < 3);
}

static class OpCodeCharacters {
private final int opcode;
private final boolean path;

OpCodeCharacters(int opcode, boolean path) {
this.opcode = opcode;
this.path = path;
}

public int getOpcode() {
return opcode;
}

public boolean isPath() {
return path;
}
}

static OpCodeCharacters[] OPCODES = new OpCodeCharacters[] {
new OpCodeCharacters(notification, false),
new OpCodeCharacters(create, true),
new OpCodeCharacters(delete, true),
new OpCodeCharacters(exists, true),
new OpCodeCharacters(getData, true),
new OpCodeCharacters(setData, true),
new OpCodeCharacters(getACL, true),
new OpCodeCharacters(setACL, true),
new OpCodeCharacters(getChildren, true),
new OpCodeCharacters(sync, true),
new OpCodeCharacters(ping, false),
new OpCodeCharacters(getChildren2, true),
new OpCodeCharacters(check, true),
new OpCodeCharacters(multi, false),
new OpCodeCharacters(create2, true),
new OpCodeCharacters(reconfig, true),
new OpCodeCharacters(checkWatches, true),
new OpCodeCharacters(removeWatches, true),
new OpCodeCharacters(createContainer, true),
new OpCodeCharacters(deleteContainer, true),
new OpCodeCharacters(createTTL, true),
new OpCodeCharacters(multiRead, false),
new OpCodeCharacters(auth, false),
new OpCodeCharacters(setWatches, false),
new OpCodeCharacters(sasl, false),
new OpCodeCharacters(getEphemerals, true),
new OpCodeCharacters(getAllChildrenNumber, true),
new OpCodeCharacters(setWatches2, false),
new OpCodeCharacters(addWatch, true),
new OpCodeCharacters(whoAmI, false),
new OpCodeCharacters(createSession, false),
new OpCodeCharacters(closeSession, false),
new OpCodeCharacters(error, false),
};

@Test
public void testOpCodeRequestPath() throws Exception {
RequestPathMetricsCollector requestPathMetricsCollector = new RequestPathMetricsCollector();
Map<Integer, OpCodeCharacters> opcodes = Arrays.stream(OPCODES).collect(HashMap::new, (map, opcode) -> map.put(opcode.getOpcode(), opcode), HashMap::putAll);

Class<?> clazz = ZooDefs.OpCode.class;
Field[] fields = clazz.getFields();

int n = 0;
for (Field field : fields) {
int opCode = field.getInt(null);
String opString = Request.op2String(opCode);

OpCodeCharacters opCodeCharacters = opcodes.get(opCode);
// Fails missing opcode to gain attention.
assertNotNull(opCodeCharacters, String.format("no characters for opcode %s", opString));

if (!opCodeCharacters.isPath()) {
continue;
}

n += 1;

String path = "/path/" + opString;
IntStream.range(0, n).forEach(ignored -> {
requestPathMetricsCollector.registerRequest(opCode, path);
});

StringWriter output = new StringWriter();
requestPathMetricsCollector.dumpTopPaths(new PrintWriter(output, true), 10);

String counts = String.format("%s : %d", path, n);
assertThat(output.toString(), containsString(counts));
}
}
}

0 comments on commit b18b7bd

Please sign in to comment.