[
https://issues.apache.org/jira/browse/LUCENE-6829?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14954520#comment-14954520
]
Dawid Weiss edited comment on LUCENE-6829 at 10/13/15 7:04 AM:
---------------------------------------------------------------
This is a large patch and I only scanned it briefly. It looks good to me. I
don't know how to avoid the virus checker special case (requiring odd hacks in
the code to disable it).
Also, blocks like this one:
{code}
+ Path tempPath =
Files.createTempDirectory(Dictionary.getDefaultTempDir(), "Hunspell");
+ boolean success = false;
+ try (Directory tempDir = FSDirectory.open(tempPath)) {
+ this.dictionary = new Dictionary(tempDir, "hunspell", affix,
dictionaries, ignoreCase);
+ success = true;
+ } finally {
+ // tempPath (directory) should be empty at this point:
+ if (success) {
+ Files.delete(tempPath);
+ } else {
+ IOUtils.deleteFilesIgnoringExceptions(tempPath);
+ }
+ }
{code}
Is there any reason why we shouldn't just let the regular exception suppression
be used here? I know it'd reverse the precedence, but at least you'd get the
full picture (temp. file couldn't be deleted too). Isn't this a leftover
pattern from before 1.7 days?
So, to be clear, why isn't the above just:
{code}
+ Path tempPath =
Files.createTempDirectory(Dictionary.getDefaultTempDir(), "Hunspell");
+ try (Directory tempDir = FSDirectory.open(tempPath)) {
+ this.dictionary = new Dictionary(tempDir, "hunspell", affix,
dictionaries, ignoreCase);
+ } finally {
+ Files.delete(tempPath); // will fail if tempPath has any files in it,
suppressing any exception.
+ }
{code}
was (Author: dweiss):
This is a large patch and I only scanned it briefly. It looks good to me. I
don't know how to avoid the virus checker special case (requiring odd hacks in
the code to disable it).
Also, blocks like this one:
{code}
+ Path tempPath =
Files.createTempDirectory(Dictionary.getDefaultTempDir(), "Hunspell");
+ boolean success = false;
+ try (Directory tempDir = FSDirectory.open(tempPath)) {
+ this.dictionary = new Dictionary(tempDir, "hunspell", affix,
dictionaries, ignoreCase);
+ success = true;
+ } finally {
+ // tempPath (directory) should be empty at this point:
+ if (success) {
+ Files.delete(tempPath);
+ } else {
+ IOUtils.deleteFilesIgnoringExceptions(tempPath);
+ }
+ }
{code}
Is there any reason why we shouldn't just let the regular exception suppression
be used here? I know it'd reverse the precedence, but at least you'd get the
full picture (temp. file couldn't be deleted too). Isn't this a leftover
pattern from before 1.7 days?
> OfflineSorter should use Directory API
> --------------------------------------
>
> Key: LUCENE-6829
> URL: https://issues.apache.org/jira/browse/LUCENE-6829
> Project: Lucene - Core
> Issue Type: Improvement
> Reporter: Michael McCandless
> Assignee: Michael McCandless
> Fix For: Trunk, 5.4
>
> Attachments: LUCENE-6829.patch, LUCENE-6829.patch, LUCENE-6829.patch,
> LUCENE-6829.patch
>
>
> I think this is a blocker for LUCENE-6825, because the block KD-tree makes
> heavy use of OfflineSorter and we don't want to fill up tmp space ...
> This should be a straightforward cutover, but there are some challenges, e.g.
> the test was failing because virus checker blocked deleting of files.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
