amit-jain commented on code in PR #1734:
URL: https://github.com/apache/jackrabbit-oak/pull/1734#discussion_r1798760253
##########
oak-run-commons/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/pipelined/PipelinedTreeStoreIT.java:
##########
@@ -675,6 +675,7 @@ private PipelinedTreeStoreStrategy
createStrategy(MongoTestBackend backend, Pred
pathPredicate,
Review Comment:
The tests miss the case of modified date which I get is difficult to test.
But can we add UT/ITs for cases like removeNode and the procedure using
updateIndexStore above?
##########
oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/incrementalstore/MergeIncrementalTreeStore.java:
##########
@@ -85,14 +99,63 @@ public String getStrategyName() {
return MERGE_BASE_AND_INCREMENTAL_TREE_STORE;
}
- /**
- * Merges multiple index store files.
- *
- * This method is a little verbose, but I think this is fine
- * as we are not getting consistent data from checkpoint diff
- * and we need to handle cases differently.
- */
- private void mergeIndexStore(File baseDir, File mergedDir) throws
IOException {
+ private static void updateIndexStore(File treeStoreDir, File
incrementalFile, Compression algorithm) throws IOException {
+ TreeStore treeStore = new TreeStore("treeStore", treeStoreDir, new
NodeStateEntryReader(null), 10);
+ long added = 0, modified = 0, upserted = 0, deleted = 0, removed = 0;
+ try (BufferedReader incrementalReader =
IndexStoreUtils.createReader(incrementalFile, algorithm)) {
+ while (true) {
+ StoreEntry line = StoreEntry.readFromReader(incrementalReader);
+ if (line == null) {
+ break;
+ }
+ String old = treeStore.getSession().get(line.path);
+ switch (line.operation) {
+ case ADD:
+ added++;
+ if (old != null) {
+ LOG.warn(
+ "ADD: node {} already exists with {}; updating
with {}",
+ line.path, old, line.value);
+ }
+ treeStore.putNode(line.path, line.value);
+ break;
+ case MODIFY:
+ modified++;
+ if (old == null) {
+ LOG.warn(
+ "MODIFY: node {} doesn't exist yet; updating
with {}",
+ line.path, line.value);
+ }
+ treeStore.putNode(line.path, line.value);
+ break;
+ case UPSERT:
+ upserted++;
+ // upsert = insert or update
+ treeStore.putNode(line.path, line.value);
+ break;
+ case DELETE:
Review Comment:
I am not really clear why are there two similar ops DELETE/REMOVE?
##########
oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/incrementalstore/MergeIncrementalTreeStore.java:
##########
@@ -67,16 +68,29 @@ public MergeIncrementalTreeStore(File baseFile, File
incrementalFile, File merge
@Override
public void doMerge() throws IOException {
Review Comment:
Test missing for this case where the TOPUP_FILE is added.
--
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]
