Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ZOOKEEPER-4750: Align RequestPathMetricsCollector with FinalRequestProcessor and OpCode #2166

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+142 −2
Open

ZOOKEEPER-4750: Align RequestPathMetricsCollector with FinalRequestProcessor and OpCode #2166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java
View file
Open in desktop
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
Open in desktop
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:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added only createTTL here.

  • addWatch touches no persistent data but only volatile data.
  • check itself modifies nothing. If data of the checking path is modified, it will be tracked through another opcode.
  • reconfig is here already.

Besides above, does sync considered as a write opcode ? I try to be conservative to changes.

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
Open in desktop
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));
}
}
}
Loading