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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]