> On Feb. 10, 2017, 7:59 p.m., nabarun nag wrote: > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystem.java, > > line 118 > > <https://reviews.apache.org/r/56557/diff/1/?file=1629590#file1629590line118> > > > > Hi Jason, > > I am not sure but will we ever need to reset this flag back to false. > > In a situation when it prevents the chunk from getting deleted when it is > > recovering from a shutdown during re-name. > > > > Then immediately after sometime, it is hitting this deleteFile call > > when it is not recovering from a crash during a rename. - is it suppose to > > delete the chunks then? will the file.possiblyRename > > still remain true.
It is possible that they could try to delete the file after this flag has been set, but that depends on what Lucenes logic is for renaming. If the recovering lucene node sees the same "meta" data as the previous lucene node, I would assume it would execute the same code path, to either rename again or if the renamed file exists, to delete it. If that is not the case we have these lingering chunks for the one file. We will probably need a way to clean out these chunks eventually but again, this is a very rare scenario and I think we would rather not have a corrupt index than worry about some minor chunks lingering? We should never have to reset this flag back to false... when would we do this? After we put the new file? (Once we put the new file we do a remove so... it's adding an operation before the remove which if remove was not invoked anyways, the new operation to reset the flag would not be invoked either) >From a Lucene perspective.. How do they do a rename? I assume they copy they >contents and write to disk and then remove the old copy. Ours will now behave >very similarly it just happens that we shared chunks instead of copying chunks. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56557/#review165159 ----------------------------------------------------------- On Feb. 10, 2017, 7:42 p.m., Jason Huynh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56557/ > ----------------------------------------------------------- > > (Updated Feb. 10, 2017, 7:42 p.m.) > > > Review request for geode, Lynn Hughes-Godfrey, nabarun nag, Dan Smith, and > xiaojian zhou. > > > Repository: geode > > > Description > ------- > > Set a possiblyRenamed field and update the file first > Any recovering node that ends up removing the renamed file should no longer > delete it's chunks > > The testPartialRename should probably be converted to seperate tests, it > actually runs differently in a debugger. Right now it expects to kill the > cache after the duplicate file has been created. So the number of operations > has to be high enough to get to the putIfAbsent in the > FileSystem.renameFile() method. > > > Diffs > ----- > > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/File.java > f3718a8 > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystem.java > 78a5b80 > > geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystemJUnitTest.java > b10b32a > > Diff: https://reviews.apache.org/r/56557/diff/ > > > Testing > ------- > > geode-lucene:precheckin > > > Thanks, > > Jason Huynh > >
