You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by "tihom88 (via GitHub)" <gi...@apache.org> on 2023/06/07 13:56:54 UTC
[GitHub] [jackrabbit-oak] tihom88 opened a new pull request, #964: OAK-10273: Index-definition json created during oak-run reindex shoul…
tihom88 opened a new pull request, #964:
URL: https://github.com/apache/jackrabbit-oak/pull/964
…d be flushed in chunks
--
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: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] fabriziofortino commented on a diff in pull request #964: OAK-10273: Index-definition json created during oak-run reindex shoul…
Posted by "fabriziofortino (via GitHub)" <gi...@apache.org>.
fabriziofortino commented on code in PR #964:
URL: https://github.com/apache/jackrabbit-oak/pull/964#discussion_r1222994405
##########
oak-store-spi/src/test/java/org/apache/jackrabbit/oak/json/JsonSerializerTest.java:
##########
@@ -63,13 +126,59 @@ public void childOrder() throws Exception{
}
} while (reader.matches(','));
+ Assert.assertTrue(customLogs.getLogs().stream().filter(n -> n.contains("Json is not getting flushed in chunks:")).findFirst().isPresent());
+ assertEquals(expectedOrder, childNames);
+ }
+
+ @Test
+ public void jsonFlushedInChunks() throws Exception {
+ System.setProperty(JsonSerializer.WRITER_FLUSH_THRESHOLD_KEY, "" + 10);
+ NodeBuilder builder = EMPTY_NODE.builder();
+ builder.child("a");
+ builder.child("b");
+ builder.child("c");
+ builder.child("d");
+
+ List<String> expectedOrder = Arrays.asList("a", "c", "b", "d");
+ builder.setProperty(":childOrder", expectedOrder, Type.NAMES);
+
+ NodeState state = builder.getNodeState();
+ String json = serializeJsonFlushedInChunks(state);
+ JsopReader reader = new JsopTokenizer(json);
+ List<String> childNames = Lists.newArrayList();
+ reader.read('{');
+ do {
+ String key = reader.readString();
+ reader.read(':');
+ if (reader.matches('{')) {
+ childNames.add(key);
+ reader.matches('}');
+ }
+
+ } while (reader.matches(','));
+ Assert.assertFalse(customLogs.getLogs().stream().filter(n -> n.contains("Json is not getting flushed in chunks:")).findFirst().isPresent());
Review Comment:
same here
```suggestion
Assert.assertFalse(customLogs.getLogs().stream().anyMatch(n -> n.contains("Json is not getting flushed in chunks:")));```
##########
oak-store-spi/src/main/java/org/apache/jackrabbit/oak/json/JsonSerializer.java:
##########
@@ -79,17 +79,30 @@ public class JsonSerializer {
private final BlobSerializer blobs;
private final boolean catchExceptions;
+ private final PrintWriter printWriter;
+ public static final String WRITER_FLUSH_THRESHOLD_KEY = "oak.writerFlushThreshold";
+
+ private final long defaultWriterFlushThreshold = 100*1024*1024L; // 100 MB
Review Comment:
minor: improve code formatting
```suggestion
private final long defaultWriterFlushThreshold = 100 * 1024 * 1024L; // 100 MB
```
##########
oak-store-spi/src/main/java/org/apache/jackrabbit/oak/json/JsonSerializer.java:
##########
@@ -184,6 +202,13 @@ public void serialize(NodeState node, String basePath) {
}
json.endObject();
+ if (printWriter != null && json instanceof JsopBuilder && ((JsopBuilder) json).length() >= writerFlushThresholdBytes) {
+ printWriter.print(json);
+ printWriter.flush();
+ ((JsopBuilder) json).flushWriter();
+ } else if (printWriter == null || !(json instanceof JsopBuilder)) {
Review Comment:
should we remove the `if` and leave just the `else`?
##########
oak-store-spi/src/test/java/org/apache/jackrabbit/oak/json/JsonSerializerTest.java:
##########
@@ -63,13 +126,59 @@ public void childOrder() throws Exception{
}
} while (reader.matches(','));
+ Assert.assertTrue(customLogs.getLogs().stream().filter(n -> n.contains("Json is not getting flushed in chunks:")).findFirst().isPresent());
Review Comment:
this could be simplified
```suggestion
Assert.assertTrue(customLogs.getLogs().stream().anyMatch(n -> n.contains("Json is not getting flushed in chunks:")));
```
##########
oak-store-spi/src/test/java/org/apache/jackrabbit/oak/json/JsonSerializerTest.java:
##########
@@ -63,13 +126,59 @@ public void childOrder() throws Exception{
}
} while (reader.matches(','));
+ Assert.assertTrue(customLogs.getLogs().stream().filter(n -> n.contains("Json is not getting flushed in chunks:")).findFirst().isPresent());
+ assertEquals(expectedOrder, childNames);
+ }
+
+ @Test
+ public void jsonFlushedInChunks() throws Exception {
+ System.setProperty(JsonSerializer.WRITER_FLUSH_THRESHOLD_KEY, "" + 10);
+ NodeBuilder builder = EMPTY_NODE.builder();
+ builder.child("a");
+ builder.child("b");
+ builder.child("c");
+ builder.child("d");
+
+ List<String> expectedOrder = Arrays.asList("a", "c", "b", "d");
+ builder.setProperty(":childOrder", expectedOrder, Type.NAMES);
+
+ NodeState state = builder.getNodeState();
+ String json = serializeJsonFlushedInChunks(state);
+ JsopReader reader = new JsopTokenizer(json);
+ List<String> childNames = Lists.newArrayList();
+ reader.read('{');
+ do {
+ String key = reader.readString();
+ reader.read(':');
+ if (reader.matches('{')) {
+ childNames.add(key);
+ reader.matches('}');
+ }
+
+ } while (reader.matches(','));
+ Assert.assertFalse(customLogs.getLogs().stream().filter(n -> n.contains("Json is not getting flushed in chunks:")).findFirst().isPresent());
assertEquals(expectedOrder, childNames);
}
- private String serialize(NodeState nodeState){
+ private String serialize(NodeState nodeState) {
JsopBuilder json = new JsopBuilder();
new JsonSerializer(json, "{\"properties\":[\"*\", \"-:*\"]}", new BlobSerializer()).serialize(nodeState);
return json.toString();
}
+
+ private String serializeJsonFlushedInChunks(NodeState nodeState) throws IOException {
+ PrintWriter pw;
+ OutputStream writerStream;
+ File outFile = temporaryFolder.newFile();
+ OutputStream os = new BufferedOutputStream(FileUtils.openOutputStream(outFile));
+ writerStream = os;
+ pw = new PrintWriter(writerStream);
Review Comment:
the `os` variable is redundant. Make it more concise
```suggestion
File outFile = temporaryFolder.newFile();
OutputStream writerStream = new BufferedOutputStream(FileUtils.openOutputStream(outFile));
PrintWriter pw = new PrintWriter(writerStream);
```
--
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: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] tihom88 commented on pull request #964: OAK-10273: Index-definition json created during oak-run reindex shoul…
Posted by "tihom88 (via GitHub)" <gi...@apache.org>.
tihom88 commented on PR #964:
URL: https://github.com/apache/jackrabbit-oak/pull/964#issuecomment-1593409257
Closing it as changing the filters should be right solution.
--
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: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] tihom88 closed pull request #964: OAK-10273: Index-definition json created during oak-run reindex shoul…
Posted by "tihom88 (via GitHub)" <gi...@apache.org>.
tihom88 closed pull request #964: OAK-10273: Index-definition json created during oak-run reindex shoul…
URL: https://github.com/apache/jackrabbit-oak/pull/964
--
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: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org