You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2021/02/03 20:16:19 UTC
[GitHub] [hudi] vinothchandar commented on a change in pull request #2495: [HUDI-1553] Configuration and metrics for the TimelineService.
vinothchandar commented on a change in pull request #2495:
URL: https://github.com/apache/hudi/pull/2495#discussion_r569712461
##########
File path: hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java
##########
@@ -111,6 +111,12 @@
public static final String DEFAULT_EMBEDDED_TIMELINE_SERVER_ENABLED = "true";
public static final String EMBEDDED_TIMELINE_SERVER_PORT = "hoodie.embed.timeline.server.port";
public static final String DEFAULT_EMBEDDED_TIMELINE_SERVER_PORT = "0";
+ public static final String EMBEDDED_TIMELINE_SERVER_THREADS = "hoodie.embed.timeline.server.threads";
+ public static final String DEFAULT_EMBEDDED_TIMELINE_SERVER_THREADS = "-1";
+ public static final String EMBEDDED_TIMELINE_SERVER_COMPRESS_OUTPUT = "hoodie.embed.timeline.server.gzip";
+ public static final String DEFAULT_EMBEDDED_TIMELINE_COMPRESS_OUTPUT = "true";
+ public static final String EMBEDDED_TIMELINE_SERVER_USE_ASYNC = "hoodie.embed.timeline.server.async";
+ public static final String DEFAULT_EMBEDDED_TIMELINE_SERVER_ASYNC = "false";
Review comment:
I have faced the same issue with jetty before. we can just turn this to `true` by default. changes seem safe enough.
##########
File path: hudi-timeline-service/src/main/java/org/apache/hudi/timeline/service/FileSystemViewHandler.java
##########
@@ -130,26 +147,54 @@ private boolean syncIfLocalViewBehind(Context ctx) {
}
private void writeValueAsString(Context ctx, Object obj) throws JsonProcessingException {
Review comment:
can we have a top level `writeValueAsString` which does something like
```
if (async) {
writeValueAsStringAsync(ctx, obj)
} else {
writeValueAsStringSync(ctx, obj)
}
```
instead of overloading the existing method to do sync and also branching for async?
##########
File path: hudi-timeline-service/src/main/java/org/apache/hudi/timeline/service/FileSystemViewHandler.java
##########
@@ -130,26 +147,54 @@ private boolean syncIfLocalViewBehind(Context ctx) {
}
private void writeValueAsString(Context ctx, Object obj) throws JsonProcessingException {
+ if (useAsync) {
+ writeValueAsStringAsync(ctx, obj);
+ return;
+ }
+
+ HoodieTimer timer = new HoodieTimer().startTimer();
boolean prettyPrint = ctx.queryParam("pretty") != null;
long beginJsonTs = System.currentTimeMillis();
String result =
prettyPrint ? OBJECT_MAPPER.writerWithDefaultPrettyPrinter().writeValueAsString(obj) : OBJECT_MAPPER.writeValueAsString(obj);
long endJsonTs = System.currentTimeMillis();
LOG.debug("Jsonify TimeTaken=" + (endJsonTs - beginJsonTs));
ctx.result(result);
+ metricsRegistry.add("WRITE_VALUE_CNT", 1);
+ metricsRegistry.add("WRITE_VALUE_TIME", timer.endTimer());
+ }
+
+ private void writeValueAsStringAsync(Context ctx, Object obj) throws JsonProcessingException {
+ ctx.result(CompletableFuture.supplyAsync(() -> {
+ HoodieTimer timer = new HoodieTimer().startTimer();
+ boolean prettyPrint = ctx.queryParam("pretty") != null;
+ long beginJsonTs = System.currentTimeMillis();
+ try {
+ String result = prettyPrint ? OBJECT_MAPPER.writerWithDefaultPrettyPrinter().writeValueAsString(obj) : OBJECT_MAPPER.writeValueAsString(obj);
+ long endJsonTs = System.currentTimeMillis();
+ LOG.debug("Jsonify TimeTaken=" + (endJsonTs - beginJsonTs));
Review comment:
guard with `isDebugEnabled()` ? to prevent this string being built each time and thrown away?
##########
File path: hudi-timeline-service/src/main/java/org/apache/hudi/timeline/service/FileSystemViewHandler.java
##########
@@ -130,26 +147,54 @@ private boolean syncIfLocalViewBehind(Context ctx) {
}
private void writeValueAsString(Context ctx, Object obj) throws JsonProcessingException {
+ if (useAsync) {
+ writeValueAsStringAsync(ctx, obj);
+ return;
+ }
+
+ HoodieTimer timer = new HoodieTimer().startTimer();
boolean prettyPrint = ctx.queryParam("pretty") != null;
long beginJsonTs = System.currentTimeMillis();
String result =
prettyPrint ? OBJECT_MAPPER.writerWithDefaultPrettyPrinter().writeValueAsString(obj) : OBJECT_MAPPER.writeValueAsString(obj);
long endJsonTs = System.currentTimeMillis();
LOG.debug("Jsonify TimeTaken=" + (endJsonTs - beginJsonTs));
ctx.result(result);
+ metricsRegistry.add("WRITE_VALUE_CNT", 1);
+ metricsRegistry.add("WRITE_VALUE_TIME", timer.endTimer());
+ }
+
+ private void writeValueAsStringAsync(Context ctx, Object obj) throws JsonProcessingException {
+ ctx.result(CompletableFuture.supplyAsync(() -> {
+ HoodieTimer timer = new HoodieTimer().startTimer();
+ boolean prettyPrint = ctx.queryParam("pretty") != null;
+ long beginJsonTs = System.currentTimeMillis();
+ try {
+ String result = prettyPrint ? OBJECT_MAPPER.writerWithDefaultPrettyPrinter().writeValueAsString(obj) : OBJECT_MAPPER.writeValueAsString(obj);
+ long endJsonTs = System.currentTimeMillis();
Review comment:
lets please use `HoodieTimer` always for timing.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org