[jira] Commented: (LUCENE-2818) abort() method for IndexOutput
[ https://issues.apache.org/jira/browse/LUCENE-2818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12972763#action_12972763 ] Michael McCandless commented on LUCENE-2818: +1 I think this'd be a good simplification of IW/IR code. I don't mind that IO would know how to delete the partial file it had created; that seems fair. So eg CompoundFileWriter would abort its output file on hitting any exception. I think we can make a default impl that simply closes suppresses exceptions? (We can't .deleteFile since an abstract IO doesn't know its Dir). Our concrete impls can override w/ versions that do delete the file... abort() method for IndexOutput -- Key: LUCENE-2818 URL: https://issues.apache.org/jira/browse/LUCENE-2818 Project: Lucene - Java Issue Type: Improvement Reporter: Earwin Burrfoot I'd like to see abort() method on IndexOutput that silently (no exceptions) closes IO and then does silent papaDir.deleteFile(this.fileName()). This will simplify a bunch of error recovery code for IndexWriter and friends, but constitutes an API backcompat break. What do you think? -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] Commented: (LUCENE-2818) abort() method for IndexOutput
[ https://issues.apache.org/jira/browse/LUCENE-2818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12972764#action_12972764 ] Earwin Burrfoot commented on LUCENE-2818: - bq. Can abort() have a default impl in IndexOutput, such as close() followed by deleteFile() maybe? If so, then it won't break anything. It can't. To call deleteFile you need both a reference to papa-Directory and a name of the file this IO writes to. Abstract IO class has neither. If we add them, they have to be passed to a new constructor, and that's an API break ;) bq. Would abort() on Directory fit better? E.g., it can abort all currently open and modified files, instead of the caller calling abort() on each IndexOutput? Are you thinking of a case where a write failed, and the caller would call abort() immediately, instead of some higher-level code? If so, would rollback() be a better name? Oh, no, no. No way. I don't want to push someone else's responsibility on Directory. This abort() is merely a shortcut. Let's go with a usage example: Here's FieldsWriter.java with LUCENE-2814 applied (skipping irrelevant parts) - https://gist.github.com/746358 Now, the same, with abort() - https://gist.github.com/746367 abort() method for IndexOutput -- Key: LUCENE-2818 URL: https://issues.apache.org/jira/browse/LUCENE-2818 Project: Lucene - Java Issue Type: Improvement Reporter: Earwin Burrfoot I'd like to see abort() method on IndexOutput that silently (no exceptions) closes IO and then does silent papaDir.deleteFile(this.fileName()). This will simplify a bunch of error recovery code for IndexWriter and friends, but constitutes an API backcompat break. What do you think? -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] Commented: (LUCENE-2818) abort() method for IndexOutput
[ https://issues.apache.org/jira/browse/LUCENE-2818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12972765#action_12972765 ] Earwin Burrfoot commented on LUCENE-2818: - bq. I think we can make a default impl that simply closes suppresses exceptions? (We can't .deleteFile since an abstract IO doesn't know its Dir). Our concrete impls can override w/ versions that do delete the file... I don't think we need a default impl? For some directory impls close() is a noop + what is more important, having abstract method forces you to implement it, you can't forget this, so we're not gonna see broken directories that don't do abort() properly. abort() method for IndexOutput -- Key: LUCENE-2818 URL: https://issues.apache.org/jira/browse/LUCENE-2818 Project: Lucene - Java Issue Type: Improvement Reporter: Earwin Burrfoot I'd like to see abort() method on IndexOutput that silently (no exceptions) closes IO and then does silent papaDir.deleteFile(this.fileName()). This will simplify a bunch of error recovery code for IndexWriter and friends, but constitutes an API backcompat break. What do you think? -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] Commented: (LUCENE-2818) abort() method for IndexOutput
[ https://issues.apache.org/jira/browse/LUCENE-2818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12972772#action_12972772 ] Shai Erera commented on LUCENE-2818: I offered a default impl just to not break the API. I don't think a default impl is a good option. If we're ok making an exception for 3x as well (I know I am), then I don't think we should have a default impl. abort() method for IndexOutput -- Key: LUCENE-2818 URL: https://issues.apache.org/jira/browse/LUCENE-2818 Project: Lucene - Java Issue Type: Improvement Reporter: Earwin Burrfoot Priority: Minor I'd like to see abort() method on IndexOutput that silently (no exceptions) closes IO and then does silent papaDir.deleteFile(this.fileName()). This will simplify a bunch of error recovery code for IndexWriter and friends, but constitutes an API backcompat break. What do you think? -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] Commented: (LUCENE-2818) abort() method for IndexOutput
[ https://issues.apache.org/jira/browse/LUCENE-2818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12972816#action_12972816 ] Michael McCandless commented on LUCENE-2818: I think a bw compat exception is fine too! abort() method for IndexOutput -- Key: LUCENE-2818 URL: https://issues.apache.org/jira/browse/LUCENE-2818 Project: Lucene - Java Issue Type: Improvement Reporter: Earwin Burrfoot Priority: Minor I'd like to see abort() method on IndexOutput that silently (no exceptions) closes IO and then does silent papaDir.deleteFile(this.fileName()). This will simplify a bunch of error recovery code for IndexWriter and friends, but constitutes an API backcompat break. What do you think? -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] Commented: (LUCENE-2818) abort() method for IndexOutput
[ https://issues.apache.org/jira/browse/LUCENE-2818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12972737#action_12972737 ] Shai Erera commented on LUCENE-2818: bq. but constitutes an API backcompat break Can abort() have a default impl in IndexOutput, such as close() followed by deleteFile() maybe? If so, then it won't break anything. Anyway, I think we can make an exception in this case - only those who impl Directory and provide their own IndexOutput extension will be affected, which I think is a relatively low number of applications? bq. What do you think? Would abort() on Directory fit better? E.g., it can abort all currently open and modified files, instead of the caller calling abort() on each IndexOutput? Are you thinking of a case where a write failed, and the caller would call abort() immediately, instead of some higher-level code? If so, would rollback() be a better name? I always thought of IndexOutput as a means for writing bytes, no special semantic logic coded and executed by it. The management code IMO should be maintained by higher-level code, such as Directory or even higher (today IndexWriter, but that's what you're trying to remove :)). So on one hand, I'd like to see IndexWriter's code simplified (this class has become a monster), but on the other, it doesn't feel right to me to add this logic in IndexOutput. Maybe I don't understand the use case for it well though. I do think though, that abort() on IndexOutput has a specific, clearer, meaning, where on Directory it can be perceived as kinda vague (what exactly is it aborting, reading / writing?). And maybe aborting a Directory is not good, if say you want to abort/rollback the changes done to a particular file. All in all, I'm +1 for simplifying IW, but am still +0 on transferring the logic to IndexOutput, unless I misunderstand the use case. abort() method for IndexOutput -- Key: LUCENE-2818 URL: https://issues.apache.org/jira/browse/LUCENE-2818 Project: Lucene - Java Issue Type: Improvement Reporter: Earwin Burrfoot I'd like to see abort() method on IndexOutput that silently (no exceptions) closes IO and then does silent papaDir.deleteFile(this.fileName()). This will simplify a bunch of error recovery code for IndexWriter and friends, but constitutes an API backcompat break. What do you think? -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
