You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Dan Smith <ds...@pivotal.io> on 2016/05/31 19:16:08 UTC
Review Request 48086: GEODE-11: Adding a tool to dump the lucene
index files
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48086/
-----------------------------------------------------------
Review request for geode, Jason Huynh, nabarun nag, and xiaojian zhou.
Repository: geode
Description
-------
Adding a function that will dump all of the files for the lucene index
to disk, for examination with external tools like Luke.
Diffs
-----
geode-junit/src/main/java/com/gemstone/gemfire/test/junit/rules/DiskDirRule.java 184619fe2332038b80d4769e88968023f6c55d63
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImpl.java 1f011a6b8650f98fd42fef71bd1593080fe91379
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFiles.java PRE-CREATION
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/File.java 2937af54631ed593ca9a5213dbcc6c964895731d
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystem.java fdafcbecfc95ec4bb9e165d7073ebbcd783ca8ae
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepository.java f1e63e096208724b935649f102f37a0be3245822
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImpl.java 3dbbc9415e176cdc8f0d9d02dc48aa93ddd20d3d
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesIntegrationTest.java PRE-CREATION
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesJUnitTest.java PRE-CREATION
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystemJUnitTest.java 1d936ca216bf06550e6ce552bdefa999a3c25a31
Diff: https://reviews.apache.org/r/48086/diff/
Testing
-------
Thanks,
Dan Smith
Re: Review Request 48086: GEODE-11: Adding a tool to dump the lucene
index files
Posted by Xiaojian Zhou <zh...@gmail.com>.
It will be even better to provide it as a public API, then the real
implementation could be put into LuceneIndexForPartitionedRegion.
Just be conservative, I suggested to put into LuceneIndexImpl as a internal
API. What I really want is a public API.
On Tue, May 31, 2016 at 4:28 PM, Dan Smith <ds...@pivotal.io> wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48086/
>
> On May 31st, 2016, 11:01 p.m. UTC, *xiaojian zhou* wrote:
>
>
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesIntegrationTest.java
> <https://reviews.apache.org/r/48086/diff/1/?file=1402210#file1402210line49> (Diff
> revision 1)
>
> 49
>
> ResultCollector resultCollector= FunctionService
>
> This feature will convert Gemfire file into java file for index. It's very useful.
>
> I think we should implement a method "convertToFile()" in LuceneIndexImpl to do this function execution.
>
> Then I can call:
> myIndex.convertToFile(diskDir) without know the detail syntax of FunctionService.
>
> BTW, I think it can be shipped.
>
> My above suggestion can be done in step2 after checked in current code.
>
> Seems like a good suggestion - are you thinking it should be part of the public API (LuceneIndex interface), or just a method on the internal class?
>
>
> - Dan
>
> On May 31st, 2016, 7:16 p.m. UTC, Dan Smith wrote:
> Review request for geode, Jason Huynh, nabarun nag, and xiaojian zhou.
> By Dan Smith.
>
> *Updated May 31, 2016, 7:16 p.m.*
> *Repository: * geode
> Description
>
> Adding a function that will dump all of the files for the lucene index
> to disk, for examination with external tools like Luke.
>
> Diffs
>
> - geode-junit/src/main/java/com/gemstone/gemfire/test/junit/rules/DiskDirRule.java
> (184619fe2332038b80d4769e88968023f6c55d63)
> - geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImpl.java
> (1f011a6b8650f98fd42fef71bd1593080fe91379)
> - geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFiles.java
> (PRE-CREATION)
> - geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/File.java
> (2937af54631ed593ca9a5213dbcc6c964895731d)
> - geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystem.java
> (fdafcbecfc95ec4bb9e165d7073ebbcd783ca8ae)
> - geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepository.java
> (f1e63e096208724b935649f102f37a0be3245822)
> - geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImpl.java
> (3dbbc9415e176cdc8f0d9d02dc48aa93ddd20d3d)
> - geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesIntegrationTest.java
> (PRE-CREATION)
> - geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesJUnitTest.java
> (PRE-CREATION)
> - geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystemJUnitTest.java
> (1d936ca216bf06550e6ce552bdefa999a3c25a31)
>
> View Diff <https://reviews.apache.org/r/48086/diff/>
>
Re: Review Request 48086: GEODE-11: Adding a tool to dump the lucene
index files
Posted by Dan Smith <ds...@pivotal.io>.
> On May 31, 2016, 11:01 p.m., xiaojian zhou wrote:
> > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesIntegrationTest.java, line 49
> > <https://reviews.apache.org/r/48086/diff/1/?file=1402210#file1402210line49>
> >
> > This feature will convert Gemfire file into java file for index. It's very useful.
> >
> > I think we should implement a method "convertToFile()" in LuceneIndexImpl to do this function execution.
> >
> > Then I can call:
> > myIndex.convertToFile(diskDir) without know the detail syntax of FunctionService.
> >
> > BTW, I think it can be shipped.
> >
> > My above suggestion can be done in step2 after checked in current code.
Seems like a good suggestion - are you thinking it should be part of the public API (LuceneIndex interface), or just a method on the internal class?
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48086/#review135709
-----------------------------------------------------------
On May 31, 2016, 7:16 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48086/
> -----------------------------------------------------------
>
> (Updated May 31, 2016, 7:16 p.m.)
>
>
> Review request for geode, Jason Huynh, nabarun nag, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adding a function that will dump all of the files for the lucene index
> to disk, for examination with external tools like Luke.
>
>
> Diffs
> -----
>
> geode-junit/src/main/java/com/gemstone/gemfire/test/junit/rules/DiskDirRule.java 184619fe2332038b80d4769e88968023f6c55d63
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImpl.java 1f011a6b8650f98fd42fef71bd1593080fe91379
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFiles.java PRE-CREATION
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/File.java 2937af54631ed593ca9a5213dbcc6c964895731d
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystem.java fdafcbecfc95ec4bb9e165d7073ebbcd783ca8ae
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepository.java f1e63e096208724b935649f102f37a0be3245822
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImpl.java 3dbbc9415e176cdc8f0d9d02dc48aa93ddd20d3d
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesIntegrationTest.java PRE-CREATION
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesJUnitTest.java PRE-CREATION
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystemJUnitTest.java 1d936ca216bf06550e6ce552bdefa999a3c25a31
>
> Diff: https://reviews.apache.org/r/48086/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 48086: GEODE-11: Adding a tool to dump the lucene
index files
Posted by xiaojian zhou <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48086/#review135709
-----------------------------------------------------------
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesIntegrationTest.java (line 49)
<https://reviews.apache.org/r/48086/#comment200756>
This feature will convert Gemfire file into java file for index. It's very useful.
I think we should implement a method "convertToFile()" in LuceneIndexImpl to do this function execution.
Then I can call:
myIndex.convertToFile(diskDir) without know the detail syntax of FunctionService.
BTW, I think it can be shipped.
My above suggestion can be done in step2 after checked in current code.
- xiaojian zhou
On May 31, 2016, 7:16 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48086/
> -----------------------------------------------------------
>
> (Updated May 31, 2016, 7:16 p.m.)
>
>
> Review request for geode, Jason Huynh, nabarun nag, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adding a function that will dump all of the files for the lucene index
> to disk, for examination with external tools like Luke.
>
>
> Diffs
> -----
>
> geode-junit/src/main/java/com/gemstone/gemfire/test/junit/rules/DiskDirRule.java 184619fe2332038b80d4769e88968023f6c55d63
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImpl.java 1f011a6b8650f98fd42fef71bd1593080fe91379
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFiles.java PRE-CREATION
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/File.java 2937af54631ed593ca9a5213dbcc6c964895731d
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystem.java fdafcbecfc95ec4bb9e165d7073ebbcd783ca8ae
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepository.java f1e63e096208724b935649f102f37a0be3245822
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImpl.java 3dbbc9415e176cdc8f0d9d02dc48aa93ddd20d3d
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesIntegrationTest.java PRE-CREATION
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesJUnitTest.java PRE-CREATION
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystemJUnitTest.java 1d936ca216bf06550e6ce552bdefa999a3c25a31
>
> Diff: https://reviews.apache.org/r/48086/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 48086: GEODE-11: Adding a tool to dump the lucene
index files
Posted by Dan Smith <ds...@pivotal.io>.
> On May 31, 2016, 9:19 p.m., Jason Huynh wrote:
> > Not sure how important it will be but maybe a test for an invalid function argument (such as a bad location or a null location?)
Sounds good, I'll add one
> On May 31, 2016, 9:19 p.m., Jason Huynh wrote:
> > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFiles.java, line 74
> > <https://reviews.apache.org/r/48086/diff/1/?file=1402205#file1402205line74>
> >
> > Maybe check to see if index is null first?
Good point, I'll fix it.
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48086/#review135684
-----------------------------------------------------------
On May 31, 2016, 7:16 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48086/
> -----------------------------------------------------------
>
> (Updated May 31, 2016, 7:16 p.m.)
>
>
> Review request for geode, Jason Huynh, nabarun nag, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adding a function that will dump all of the files for the lucene index
> to disk, for examination with external tools like Luke.
>
>
> Diffs
> -----
>
> geode-junit/src/main/java/com/gemstone/gemfire/test/junit/rules/DiskDirRule.java 184619fe2332038b80d4769e88968023f6c55d63
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImpl.java 1f011a6b8650f98fd42fef71bd1593080fe91379
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFiles.java PRE-CREATION
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/File.java 2937af54631ed593ca9a5213dbcc6c964895731d
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystem.java fdafcbecfc95ec4bb9e165d7073ebbcd783ca8ae
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepository.java f1e63e096208724b935649f102f37a0be3245822
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImpl.java 3dbbc9415e176cdc8f0d9d02dc48aa93ddd20d3d
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesIntegrationTest.java PRE-CREATION
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesJUnitTest.java PRE-CREATION
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystemJUnitTest.java 1d936ca216bf06550e6ce552bdefa999a3c25a31
>
> Diff: https://reviews.apache.org/r/48086/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 48086: GEODE-11: Adding a tool to dump the lucene
index files
Posted by Jason Huynh <hu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48086/#review135684
-----------------------------------------------------------
Not sure how important it will be but maybe a test for an invalid function argument (such as a bad location or a null location?)
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFiles.java (line 74)
<https://reviews.apache.org/r/48086/#comment200702>
Maybe check to see if index is null first?
- Jason Huynh
On May 31, 2016, 7:16 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48086/
> -----------------------------------------------------------
>
> (Updated May 31, 2016, 7:16 p.m.)
>
>
> Review request for geode, Jason Huynh, nabarun nag, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Adding a function that will dump all of the files for the lucene index
> to disk, for examination with external tools like Luke.
>
>
> Diffs
> -----
>
> geode-junit/src/main/java/com/gemstone/gemfire/test/junit/rules/DiskDirRule.java 184619fe2332038b80d4769e88968023f6c55d63
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneServiceImpl.java 1f011a6b8650f98fd42fef71bd1593080fe91379
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFiles.java PRE-CREATION
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/File.java 2937af54631ed593ca9a5213dbcc6c964895731d
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystem.java fdafcbecfc95ec4bb9e165d7073ebbcd783ca8ae
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepository.java f1e63e096208724b935649f102f37a0be3245822
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImpl.java 3dbbc9415e176cdc8f0d9d02dc48aa93ddd20d3d
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesIntegrationTest.java PRE-CREATION
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/directory/DumpDirectoryFilesJUnitTest.java PRE-CREATION
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystemJUnitTest.java 1d936ca216bf06550e6ce552bdefa999a3c25a31
>
> Diff: https://reviews.apache.org/r/48086/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>