You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Dawid Weiss (JIRA)" <ji...@apache.org> on 2015/10/13 09:05:05 UTC
[jira] [Comment Edited] (LUCENE-6829) OfflineSorter should use
Directory API
[ 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org