[
https://issues.apache.org/jira/browse/OAK-3547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15551315#comment-15551315
]
Chetan Mehrotra edited comment on OAK-3547 at 10/6/16 8:28 AM:
---------------------------------------------------------------
Thanks [~ianeboston] for the feature patch. Some feedback below on current
approach
List of NodeState using generational approach [1]
{noformat}
Base state - No file added
===========================
/{saveDirectoryListing = true}
:data
:dir
l_1475740652371{state = }
l_1475740652360{state = }
:data
===========================
3 files added
===========================
/{saveDirectoryListing = true}
:data
:dir
l_1475740652371{state = }
l_1475740652394{state =
foo2,foo2,612,5b41ac56a8454e48af2dd7aa740f71a96a740366;foo1,foo1,233,ff0e1a49171201d638848c3f3e9b003596a81d6b;foo0,foo0,73,7feabefe2276000fa7722d6536183a508816d2c7;}
l_1475740652360{state = }
:data
foo2{blobSize = 1047552, jcr:lastModified = 1475740652394, jcr:data = [1
binaries], uniqueKey = 84dba956e1634dc14812db9a7ae6a81c}
foo1{blobSize = 1047552, jcr:lastModified = 1475740652393, jcr:data = [1
binaries], uniqueKey = a9505647e35a7f0e0365adbc2531c91f}
foo0{blobSize = 1047552, jcr:lastModified = 1475740652392, jcr:data = [1
binaries], uniqueKey = 937a0057a90df312f452aceb96f40c8c}
===========================
3 'bar' files added
===========================
/{saveDirectoryListing = true}
:data
:dir
l_1475740652371{state = }
l_1475740652407{state =
foo2,foo2,612,5b41ac56a8454e48af2dd7aa740f71a96a740366;bar2,bar2,656,2ea3ee49550e65f45d2e8d706e1a3bcef2d4a8b3;foo1,foo1,233,ff0e1a49171201d638848c3f3e9b003596a81d6b;bar1,bar1,953,a296f858ccb93414d6e99ca7a1997fb711faa65d;foo0,foo0,73,7feabefe2276000fa7722d6536183a508816d2c7;bar0,bar0,433,83ad0b2e68f114aa901c9eee0715c4507cd5dfe1;}
l_1475740652394{state =
foo2,foo2,612,5b41ac56a8454e48af2dd7aa740f71a96a740366;foo1,foo1,233,ff0e1a49171201d638848c3f3e9b003596a81d6b;foo0,foo0,73,7feabefe2276000fa7722d6536183a508816d2c7;}
l_1475740652360{state = }
:data
foo2{blobSize = 1047552, jcr:lastModified = 1475740652394, jcr:data = [1
binaries], uniqueKey = 84dba956e1634dc14812db9a7ae6a81c}
foo1{blobSize = 1047552, jcr:lastModified = 1475740652393, jcr:data = [1
binaries], uniqueKey = a9505647e35a7f0e0365adbc2531c91f}
foo0{blobSize = 1047552, jcr:lastModified = 1475740652392, jcr:data = [1
binaries], uniqueKey = 937a0057a90df312f452aceb96f40c8c}
bar1{blobSize = 1047552, jcr:lastModified = 1475740652406, jcr:data = [1
binaries], uniqueKey = 5ff21364ce67199cddd14833f0614d73}
bar2{blobSize = 1047552, jcr:lastModified = 1475740652406, jcr:data = [1
binaries], uniqueKey = c10e5f46e90e01d927f0aee82980ac6d}
bar0{blobSize = 1047552, jcr:lastModified = 1475740652405, jcr:data = [1
binaries], uniqueKey = 0f32d06997a4fec9a90060b863cda454}
===========================
{noformat}
* Empty {{:data}} created under {{:data}}. Is this required or possibly due bug
below. In line #2 same builder should be used
{code}
private SimpleDirectoryListing(@Nonnull IndexDefinition definition,
@Nonnull NodeBuilder builder) {
this.definition = definition;
this.directoryBuilder = getOrCreateChild(builder,
INDEX_DATA_CHILD_NAME);
this.fileNames.addAll(getListing());
}
{code}
* load and save is called every time even if no change is done. This adds empty
l_ddd nodes. This should be avoided
* LISTING_STATE_PROPERTY -
** It holds an encoded listing info stored as single string property. Hopefully
this does not grow very large if directory content is large. Not sure of
typical sizes
** You can possibly use a MultivalueProperty here.
* Not sure on below snippet in {{load}} method.
{code}
if (loaded >= 0 ) {
return (loaded != childNodes.size() - 1);
}
{code}
* {{doGC}} and {{sync}} methods do not have any test coverage
h4. Feature Flag
It would be more comforting if this feature is driven by a feature flag. So
generation logic is used if enabled otherwise it defaults to
{{GenerationalDirectoryListing}}. We can expose a setting in
{{LuceneIndexProviderService}} to lock new feature so as to enable controlled
testing.
I think having flag to enable when it is not being used would be easy. What
would be tricky is to have it disabled once enabled ... that aspect can
possibly be ignored
h4. Effect of corruption on writes
Currently if the corruption occurs system would automatically fallback to older
version. This is fine for reads but for writes this would mean data loss unless
indexed. As Async indexer would only index newer stuff. We have 2 options here
# Let async indexer continue but provide some indication that index is corrupt
and reindex is required in some time - This needs to be highlighted in
prominenet way (periodic logs, JMX etc)
# Let fallback used for reads (readOnly == true) but let it fail for writes
Possibly this needs to be exposed as config option defaulting to #2 to avoid
surprises
h4. What type of corruption is being targetted
Current approach is trying to defend/detect against those corruption where
OakDirectory is used for read and write always. So once written if we are
opening a directory for IndexReader then it would proactively check that files
present in blob store are corrupted or not.
However in current usage setups make use of CopyOnRead and CopyOnWrite
directory along with OakDirectory. So we would need to have some support in
{{IndexCopier}} so as to check that files present on file system are valid wrt
expected checksum
h4. Move code to {{directory}} package
OakDirectory is getting bigger with this change. As new classes
{{GenerationalDirectoryListing}} and other are static it would be better if we
add them to directory package. This would simplify unit testing them also
[1]
{code}
@Test
public void saveListing() throws Exception{
NodeBuilder builder = EMPTY_NODE.builder();
builder.setProperty(LuceneIndexConstants.SAVE_DIR_LISTING, true);
Directory dir = createDir(builder, false);
dir.close();
dumpState(builder, "Base state - No file added");
Set<String> fileNames = newHashSet();
dir = createDir(builder, false);
createFile(dir, fileNames, "foo");
dir.close();
dumpState(builder, "3 files added");
dir = createDir(builder, false);
createFile(dir, fileNames, "bar");
dir.close();
dumpState(builder, "3 'bar' files added");
assertEquals(fileNames, newHashSet(dir.listAll()));
}
private void createFile(Directory dir, Set<String> fileNames, String
prefix) throws IOException {
for (int i = 0; i < 3; i++) {
String fileName = prefix + i;
createFile(dir, fileName);
fileNames.add(fileName);
}
}
private static void dumpState(NodeBuilder builder, String msg){
System.out.println();
System.out.println(msg);
System.out.println("===========================");
System.out.println(NodeStateUtils.toString(builder.getNodeState()));
System.out.println("===========================");
}
{code}
was (Author: chetanm):
List of NodeState using generational approach [1]
{noformat}
Base state - No file added
===========================
/{saveDirectoryListing = true}
:data
:dir
l_1475740652371{state = }
l_1475740652360{state = }
:data
===========================
3 files added
===========================
/{saveDirectoryListing = true}
:data
:dir
l_1475740652371{state = }
l_1475740652394{state =
foo2,foo2,612,5b41ac56a8454e48af2dd7aa740f71a96a740366;foo1,foo1,233,ff0e1a49171201d638848c3f3e9b003596a81d6b;foo0,foo0,73,7feabefe2276000fa7722d6536183a508816d2c7;}
l_1475740652360{state = }
:data
foo2{blobSize = 1047552, jcr:lastModified = 1475740652394, jcr:data = [1
binaries], uniqueKey = 84dba956e1634dc14812db9a7ae6a81c}
foo1{blobSize = 1047552, jcr:lastModified = 1475740652393, jcr:data = [1
binaries], uniqueKey = a9505647e35a7f0e0365adbc2531c91f}
foo0{blobSize = 1047552, jcr:lastModified = 1475740652392, jcr:data = [1
binaries], uniqueKey = 937a0057a90df312f452aceb96f40c8c}
===========================
3 'bar' files added
===========================
/{saveDirectoryListing = true}
:data
:dir
l_1475740652371{state = }
l_1475740652407{state =
foo2,foo2,612,5b41ac56a8454e48af2dd7aa740f71a96a740366;bar2,bar2,656,2ea3ee49550e65f45d2e8d706e1a3bcef2d4a8b3;foo1,foo1,233,ff0e1a49171201d638848c3f3e9b003596a81d6b;bar1,bar1,953,a296f858ccb93414d6e99ca7a1997fb711faa65d;foo0,foo0,73,7feabefe2276000fa7722d6536183a508816d2c7;bar0,bar0,433,83ad0b2e68f114aa901c9eee0715c4507cd5dfe1;}
l_1475740652394{state =
foo2,foo2,612,5b41ac56a8454e48af2dd7aa740f71a96a740366;foo1,foo1,233,ff0e1a49171201d638848c3f3e9b003596a81d6b;foo0,foo0,73,7feabefe2276000fa7722d6536183a508816d2c7;}
l_1475740652360{state = }
:data
foo2{blobSize = 1047552, jcr:lastModified = 1475740652394, jcr:data = [1
binaries], uniqueKey = 84dba956e1634dc14812db9a7ae6a81c}
foo1{blobSize = 1047552, jcr:lastModified = 1475740652393, jcr:data = [1
binaries], uniqueKey = a9505647e35a7f0e0365adbc2531c91f}
foo0{blobSize = 1047552, jcr:lastModified = 1475740652392, jcr:data = [1
binaries], uniqueKey = 937a0057a90df312f452aceb96f40c8c}
bar1{blobSize = 1047552, jcr:lastModified = 1475740652406, jcr:data = [1
binaries], uniqueKey = 5ff21364ce67199cddd14833f0614d73}
bar2{blobSize = 1047552, jcr:lastModified = 1475740652406, jcr:data = [1
binaries], uniqueKey = c10e5f46e90e01d927f0aee82980ac6d}
bar0{blobSize = 1047552, jcr:lastModified = 1475740652405, jcr:data = [1
binaries], uniqueKey = 0f32d06997a4fec9a90060b863cda454}
===========================
{noformat}
* Empty {{:data}} created under {{:data}}. Is this required or possibly due bug
below. In line #2 same builder should be used
{code}
private SimpleDirectoryListing(@Nonnull IndexDefinition definition,
@Nonnull NodeBuilder builder) {
this.definition = definition;
this.directoryBuilder = getOrCreateChild(builder,
INDEX_DATA_CHILD_NAME);
this.fileNames.addAll(getListing());
}
{code}
* load and save is called every time even if no change is done. This adds empty
l_ddd nodes. This should be avoided
* LISTING_STATE_PROPERTY -
** It holds an encoded listing info stored as single string property. Hopefully
this does not grow very large if directory content is large. Not sure of
typical sizes
** You can possibly use a MultivalueProperty here.
* Not sure on below snippet in {{load}} method.
{code}
if (loaded >= 0 ) {
return (loaded != childNodes.size() - 1);
}
{code}
* {{doGC}} and {{sync}} methods do not have any test coverage
*Feature Flag*
It would be more comforting if this feature is driven by a feature flag. So
generation logic is used if enabled otherwise it defaults to
{{GenerationalDirectoryListing}}. We can expose a setting in
{{LuceneIndexProviderService}} to lock new feature so as to enable controlled
testing.
I think having flag to enable when it is not being used would be easy. What
would be tricky is to have it disabled once enabled ... that aspect can
possibly be ignored
h5. Effect of corruption on writes
Currently if the corruption occurs system would automatically fallback to older
version. This is fine for reads but for writes this would mean data loss unless
indexed. As Async indexer would only index newer stuff. We have 2 options here
# Let async indexer continue but provide some indication that index is corrupt
and reindex is required in some time - This needs to be highlighted in
prominenet way (periodic logs, JMX etc)
# Let fallback used for reads (readOnly == true) but let it fail for writes
Possibly this needs to be exposed as config option defaulting to #2 to avoid
surprises
h5. What type of corruption is being targetted
Current approach is trying to defend/detect against those corruption where
OakDirectory is used for read and write always. So once written if we are
opening a directory for IndexReader then it would proactively check that files
present in blob store are corrupted or not.
However in current usage setups make use of CopyOnRead and CopyOnWrite
directory along with OakDirectory. So we would need to have some support in
{{IndexCopier}} so as to check that files present on file system are valid wrt
expected checksum
h5. Move code to {{directory}} package
OakDirectory is getting bigger with this change. As new classes
{{GenerationalDirectoryListing}} and other are static it would be better if we
add them to directory package. This would simplify unit testing them also
[1]
{code}
@Test
public void saveListing() throws Exception{
NodeBuilder builder = EMPTY_NODE.builder();
builder.setProperty(LuceneIndexConstants.SAVE_DIR_LISTING, true);
Directory dir = createDir(builder, false);
dir.close();
dumpState(builder, "Base state - No file added");
Set<String> fileNames = newHashSet();
dir = createDir(builder, false);
createFile(dir, fileNames, "foo");
dir.close();
dumpState(builder, "3 files added");
dir = createDir(builder, false);
createFile(dir, fileNames, "bar");
dir.close();
dumpState(builder, "3 'bar' files added");
assertEquals(fileNames, newHashSet(dir.listAll()));
}
private void createFile(Directory dir, Set<String> fileNames, String
prefix) throws IOException {
for (int i = 0; i < 3; i++) {
String fileName = prefix + i;
createFile(dir, fileName);
fileNames.add(fileName);
}
}
private static void dumpState(NodeBuilder builder, String msg){
System.out.println();
System.out.println(msg);
System.out.println("===========================");
System.out.println(NodeStateUtils.toString(builder.getNodeState()));
System.out.println("===========================");
}
{code}
> Improve ability of the OakDirectory to recover from unexpected file errors
> --------------------------------------------------------------------------
>
> Key: OAK-3547
> URL: https://issues.apache.org/jira/browse/OAK-3547
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: lucene
> Affects Versions: 1.4
> Reporter: Ian Boston
> Fix For: 1.6
>
>
> Currently if the OakDirectory finds that a file is missing or in some way
> damaged, and exception is thrown which impacts all queries using that index,
> at times making the index unavailable. This improvement aims to make the
> OakDirectory recover to a previously ok state by storing which files were
> involved in previous states, and giving the code some way of checking if they
> are valid.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
