-
Notifications
You must be signed in to change notification settings - Fork 116
Add merge functionality on Directory object #162
Conversation
|
Could you add the merge result of "v__=0" and "v__=1" simply in description? |
|
Also, please update the PR description to indicate which uber-issue this is part of. |
src/main/scala/com/microsoft/hyperspace/index/IndexLogEntry.scala
Outdated
Show resolved
Hide resolved
src/test/scala/com/microsoft/hyperspace/index/IndexLogEntryTest.scala
Outdated
Show resolved
Hide resolved
thanks @sezruby , added |
Thanks! I saw you updated the PR description. Can you link to the uber-issue please? |
thanks @rapoth , updated the PR description |
src/main/scala/com/microsoft/hyperspace/index/IndexLogEntry.scala
Outdated
Show resolved
Hide resolved
src/main/scala/com/microsoft/hyperspace/index/IndexLogEntry.scala
Outdated
Show resolved
Hide resolved
src/main/scala/com/microsoft/hyperspace/index/IndexLogEntry.scala
Outdated
Show resolved
Hide resolved
src/main/scala/com/microsoft/hyperspace/index/IndexLogEntry.scala
Outdated
Show resolved
Hide resolved
src/main/scala/com/microsoft/hyperspace/index/IndexLogEntry.scala
Outdated
Show resolved
Hide resolved
src/test/scala/com/microsoft/hyperspace/index/IndexLogEntryTest.scala
Outdated
Show resolved
Hide resolved
src/test/scala/com/microsoft/hyperspace/index/IndexLogEntryTest.scala
Outdated
Show resolved
Hide resolved
src/test/scala/com/microsoft/hyperspace/index/IndexLogEntryTest.scala
Outdated
Show resolved
Hide resolved
src/main/scala/com/microsoft/hyperspace/index/IndexLogEntry.scala
Outdated
Show resolved
Hide resolved
| // temp/a/f1 | ||
| // temp/b/f2 | ||
| // testDir/temp/a/f1 | ||
| // testDir/temp/b/f2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is an resolution of this comment. It's repeated across many other tests.
| // testDir/a/f1 | ||
| // testDir/b/c/f2 | ||
| // testDir/temp/a/f1 | ||
| // testDir/temp/b/c/f2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is an resolution of this comment. It's repeated across many other tests.
| // testDir/a/c/f3 | ||
| // testDir/temp/a/f1 | ||
| // testDir/temp/a/b/f2 | ||
| // testDir/temp/a/c/f3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is an resolution of this comment. It's repeated across many other tests.
imback82
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @apoorvedave1!
|
I rebased your branch, so make sure to run |
|
LGTM. All my comments have been resolved. Thanks a lot @apoorvedave1! |
What changes were proposed in this pull request?
Add support for
mergeing twocom.microsoft.hyperspace.index.Directoryobjects. Pre-requisite for #29, #163.Feature Description
We need a functionality to merge two directory trees in order to support future functionalities like
AppendandOptimize. When we create index only on partially unindexed data, we need to make sure to log the latest snapshot of the index as the combination of previously created index files and newly created index files. This can be done through the suggestedmergeapi on indexDirectory.Example Usecase: Merge contents of partially created indexes in case of append:
index/v__=0. This will be stored in metadata asUser adds new data and calls
refreshIndex(mode = "quick"). This will create another intermediateContentobject similar to above, withname = "v__=1".We can now call
content.root.merge(content2.root)to deduplicate the directory structure.Does this PR introduce any user-facing change?
no
How was this patch tested?
unit tests