[jira] [Commented] (LUCENE-5708) Remove IndexWriterConfig.clone
[
https://issues.apache.org/jira/browse/LUCENE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14009722#comment-14009722
]
Shai Erera commented on LUCENE-5708:
I think the way you fixed some tests that used clone is incorrect. You should
at least call {{newIndexWriterConfig(random)}} w/ the same random and seed, so
the exact IWC is created each time. At least, that's what these tests now rely
on, even if they don't break. Otherwise, they just create a random IWC each
time they open a writer, which is not the intention I believe.
> Remove IndexWriterConfig.clone
> --
>
> Key: LUCENE-5708
> URL: https://issues.apache.org/jira/browse/LUCENE-5708
> Project: Lucene - Core
> Issue Type: Bug
> Components: core/index
>Reporter: Michael McCandless
>Assignee: Michael McCandless
> Fix For: 4.9, 5.0
>
> Attachments: LUCENE-5708.patch, LUCENE-5708.patch
>
>
> We originally added this clone to allow a single IWC to be re-used against
> more than one IndexWriter, but I think this is a mis-feature: it adds
> complexity to hairy classes (merge policy/scheduler, DW thread pool, etc.), I
> think it's buggy today.
> I think we should just disallow sharing: you must make a new IWC for a new
> IndexWriter.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-5708) Remove IndexWriterConfig.clone
[ https://issues.apache.org/jira/browse/LUCENE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14009824#comment-14009824 ] Michael McCandless commented on LUCENE-5708: Hmm which tests rely on using the same IWC? I thought I was improving the tests by switching up the config... > Remove IndexWriterConfig.clone > -- > > Key: LUCENE-5708 > URL: https://issues.apache.org/jira/browse/LUCENE-5708 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Michael McCandless >Assignee: Michael McCandless > Fix For: 4.9, 5.0 > > Attachments: LUCENE-5708.patch, LUCENE-5708.patch > > > We originally added this clone to allow a single IWC to be re-used against > more than one IndexWriter, but I think this is a mis-feature: it adds > complexity to hairy classes (merge policy/scheduler, DW thread pool, etc.), I > think it's buggy today. > I think we should just disallow sharing: you must make a new IWC for a new > IndexWriter. -- This message was sent by Atlassian JIRA (v6.2#6252) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-5708) Remove IndexWriterConfig.clone
[ https://issues.apache.org/jira/browse/LUCENE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14009930#comment-14009930 ] Adrien Grand commented on LUCENE-5708: -- I don't know about these tests that expect the same config but I'm +1 in general to remove all that cloning. It looks to me that we should be able to make some fields final now that we don't have a clone method anymore (eg. MergePolicy.writer)? > Remove IndexWriterConfig.clone > -- > > Key: LUCENE-5708 > URL: https://issues.apache.org/jira/browse/LUCENE-5708 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Michael McCandless >Assignee: Michael McCandless > Fix For: 4.9, 5.0 > > Attachments: LUCENE-5708.patch, LUCENE-5708.patch > > > We originally added this clone to allow a single IWC to be re-used against > more than one IndexWriter, but I think this is a mis-feature: it adds > complexity to hairy classes (merge policy/scheduler, DW thread pool, etc.), I > think it's buggy today. > I think we should just disallow sharing: you must make a new IWC for a new > IndexWriter. -- This message was sent by Atlassian JIRA (v6.2#6252) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-5708) Remove IndexWriterConfig.clone
[ https://issues.apache.org/jira/browse/LUCENE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14009988#comment-14009988 ] Shai Erera commented on LUCENE-5708: bq. Hmm which tests rely on using the same IWC? Hmm ... I don't remember. All I remember is that while I worked on preventing sharing IWC between writers (LUCENE-4876), there were a bunch of tests that reused the IWC. I fixed them by simply cloning it, but I admit I didn't check if initializing a new IWC each time serves their purpose. I just assume that if so many tests did that, there ought to be a reason beyond just convenience, but I could be wrong. What I'm worried is that by not cloning Jenkins will trip (which is good!), or worse - that those tests will stop asserting what they asserted before. So I just wanted to point that out. If we're ready to take the risk, I'm fine with it, because eventually we're discussing tests here .. there's nothing functionally missing from an app's perspective. > Remove IndexWriterConfig.clone > -- > > Key: LUCENE-5708 > URL: https://issues.apache.org/jira/browse/LUCENE-5708 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Michael McCandless >Assignee: Michael McCandless > Fix For: 4.9, 5.0 > > Attachments: LUCENE-5708.patch, LUCENE-5708.patch > > > We originally added this clone to allow a single IWC to be re-used against > more than one IndexWriter, but I think this is a mis-feature: it adds > complexity to hairy classes (merge policy/scheduler, DW thread pool, etc.), I > think it's buggy today. > I think we should just disallow sharing: you must make a new IWC for a new > IndexWriter. -- This message was sent by Atlassian JIRA (v6.2#6252) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-5708) Remove IndexWriterConfig.clone
[ https://issues.apache.org/jira/browse/LUCENE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14011085#comment-14011085 ] Simon Willnauer commented on LUCENE-5708: - part of the problem is that we holding IW state on MergePolicy and MergeScheduler. Both classes should get the IW passed to the relevant methods so we can share them across as many instances we want... > Remove IndexWriterConfig.clone > -- > > Key: LUCENE-5708 > URL: https://issues.apache.org/jira/browse/LUCENE-5708 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Michael McCandless >Assignee: Michael McCandless > Fix For: 4.9, 5.0 > > Attachments: LUCENE-5708.patch, LUCENE-5708.patch > > > We originally added this clone to allow a single IWC to be re-used against > more than one IndexWriter, but I think this is a mis-feature: it adds > complexity to hairy classes (merge policy/scheduler, DW thread pool, etc.), I > think it's buggy today. > I think we should just disallow sharing: you must make a new IWC for a new > IndexWriter. -- This message was sent by Atlassian JIRA (v6.2#6252) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-5708) Remove IndexWriterConfig.clone
[ https://issues.apache.org/jira/browse/LUCENE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14013408#comment-14013408 ] ASF subversion and git services commented on LUCENE-5708: - Commit 1598489 from [~mikemccand] in branch 'dev/trunk' [ https://svn.apache.org/r1598489 ] LUCENE-5708: remove IWC.clone > Remove IndexWriterConfig.clone > -- > > Key: LUCENE-5708 > URL: https://issues.apache.org/jira/browse/LUCENE-5708 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Michael McCandless >Assignee: Michael McCandless > Fix For: 4.9, 5.0 > > Attachments: LUCENE-5708.patch, LUCENE-5708.patch, LUCENE-5708.patch > > > We originally added this clone to allow a single IWC to be re-used against > more than one IndexWriter, but I think this is a mis-feature: it adds > complexity to hairy classes (merge policy/scheduler, DW thread pool, etc.), I > think it's buggy today. > I think we should just disallow sharing: you must make a new IWC for a new > IndexWriter. -- This message was sent by Atlassian JIRA (v6.2#6252) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-5708) Remove IndexWriterConfig.clone
[ https://issues.apache.org/jira/browse/LUCENE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14013412#comment-14013412 ] ASF subversion and git services commented on LUCENE-5708: - Commit 1598492 from [~mikemccand] in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1598492 ] LUCENE-5708: remove IWC.clone > Remove IndexWriterConfig.clone > -- > > Key: LUCENE-5708 > URL: https://issues.apache.org/jira/browse/LUCENE-5708 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Michael McCandless >Assignee: Michael McCandless > Fix For: 4.9, 5.0 > > Attachments: LUCENE-5708.patch, LUCENE-5708.patch, LUCENE-5708.patch > > > We originally added this clone to allow a single IWC to be re-used against > more than one IndexWriter, but I think this is a mis-feature: it adds > complexity to hairy classes (merge policy/scheduler, DW thread pool, etc.), I > think it's buggy today. > I think we should just disallow sharing: you must make a new IWC for a new > IndexWriter. -- This message was sent by Atlassian JIRA (v6.2#6252) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-5708) Remove IndexWriterConfig.clone
[ https://issues.apache.org/jira/browse/LUCENE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14013424#comment-14013424 ] ASF subversion and git services commented on LUCENE-5708: - Commit 1598496 from [~mikemccand] in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1598496 ] LUCENE-5708: fix test bug > Remove IndexWriterConfig.clone > -- > > Key: LUCENE-5708 > URL: https://issues.apache.org/jira/browse/LUCENE-5708 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Michael McCandless >Assignee: Michael McCandless > Fix For: 4.9, 5.0 > > Attachments: LUCENE-5708.patch, LUCENE-5708.patch, LUCENE-5708.patch > > > We originally added this clone to allow a single IWC to be re-used against > more than one IndexWriter, but I think this is a mis-feature: it adds > complexity to hairy classes (merge policy/scheduler, DW thread pool, etc.), I > think it's buggy today. > I think we should just disallow sharing: you must make a new IWC for a new > IndexWriter. -- This message was sent by Atlassian JIRA (v6.2#6252) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-5708) Remove IndexWriterConfig.clone
[ https://issues.apache.org/jira/browse/LUCENE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14013426#comment-14013426 ] ASF subversion and git services commented on LUCENE-5708: - Commit 1598497 from [~mikemccand] in branch 'dev/trunk' [ https://svn.apache.org/r1598497 ] LUCENE-5708: fix test bug > Remove IndexWriterConfig.clone > -- > > Key: LUCENE-5708 > URL: https://issues.apache.org/jira/browse/LUCENE-5708 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Michael McCandless >Assignee: Michael McCandless > Fix For: 4.9, 5.0 > > Attachments: LUCENE-5708.patch, LUCENE-5708.patch, LUCENE-5708.patch > > > We originally added this clone to allow a single IWC to be re-used against > more than one IndexWriter, but I think this is a mis-feature: it adds > complexity to hairy classes (merge policy/scheduler, DW thread pool, etc.), I > think it's buggy today. > I think we should just disallow sharing: you must make a new IWC for a new > IndexWriter. -- This message was sent by Atlassian JIRA (v6.2#6252) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-5708) Remove IndexWriterConfig.clone
[ https://issues.apache.org/jira/browse/LUCENE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14013434#comment-14013434 ] ASF subversion and git services commented on LUCENE-5708: - Commit 1598502 from [~mikemccand] in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1598502 ] LUCENE-5708: fix test bug > Remove IndexWriterConfig.clone > -- > > Key: LUCENE-5708 > URL: https://issues.apache.org/jira/browse/LUCENE-5708 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Michael McCandless >Assignee: Michael McCandless > Fix For: 4.9, 5.0 > > Attachments: LUCENE-5708.patch, LUCENE-5708.patch, LUCENE-5708.patch > > > We originally added this clone to allow a single IWC to be re-used against > more than one IndexWriter, but I think this is a mis-feature: it adds > complexity to hairy classes (merge policy/scheduler, DW thread pool, etc.), I > think it's buggy today. > I think we should just disallow sharing: you must make a new IWC for a new > IndexWriter. -- This message was sent by Atlassian JIRA (v6.2#6252) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-5708) Remove IndexWriterConfig.clone
[ https://issues.apache.org/jira/browse/LUCENE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14013437#comment-14013437 ] ASF subversion and git services commented on LUCENE-5708: - Commit 1598503 from [~mikemccand] in branch 'dev/trunk' [ https://svn.apache.org/r1598503 ] LUCENE-5708: fix test bug > Remove IndexWriterConfig.clone > -- > > Key: LUCENE-5708 > URL: https://issues.apache.org/jira/browse/LUCENE-5708 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Michael McCandless >Assignee: Michael McCandless > Fix For: 4.9, 5.0 > > Attachments: LUCENE-5708.patch, LUCENE-5708.patch, LUCENE-5708.patch > > > We originally added this clone to allow a single IWC to be re-used against > more than one IndexWriter, but I think this is a mis-feature: it adds > complexity to hairy classes (merge policy/scheduler, DW thread pool, etc.), I > think it's buggy today. > I think we should just disallow sharing: you must make a new IWC for a new > IndexWriter. -- This message was sent by Atlassian JIRA (v6.2#6252) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-5708) Remove IndexWriterConfig.clone
[ https://issues.apache.org/jira/browse/LUCENE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14013459#comment-14013459 ] Michael McCandless commented on LUCENE-5708: bq. Tests arent testing what they should, we need to fix the disease or back this change out. I'm happy to back this change out if you want. But, I'm confused: are you talking about how these tests hardwire the PF to Lucene41? I agree we should fix that, but this is a pre-existing issue. > Remove IndexWriterConfig.clone > -- > > Key: LUCENE-5708 > URL: https://issues.apache.org/jira/browse/LUCENE-5708 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Michael McCandless >Assignee: Michael McCandless > Fix For: 4.9, 5.0 > > Attachments: LUCENE-5708.patch, LUCENE-5708.patch, LUCENE-5708.patch > > > We originally added this clone to allow a single IWC to be re-used against > more than one IndexWriter, but I think this is a mis-feature: it adds > complexity to hairy classes (merge policy/scheduler, DW thread pool, etc.), I > think it's buggy today. > I think we should just disallow sharing: you must make a new IWC for a new > IndexWriter. -- This message was sent by Atlassian JIRA (v6.2#6252) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-5708) Remove IndexWriterConfig.clone
[ https://issues.apache.org/jira/browse/LUCENE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14013477#comment-14013477 ] Robert Muir commented on LUCENE-5708: - All these "PostingsFormat" tests wire their codec to themselves. This allows us to implement generic base classes with tests that all codecs should pass, as well as codec-specific tests (e.g. in lucene41) that test particular corner cases of importance. this worked well in knocking out bugs for postings, so the whole scheme was duplicated to docvalues, storedfields, vectors, everything. Now here comes this commit, and these tests (which are important to ensure the index format is working) are no longer testing what they are supposed to. For example the tests in Lucene41 package explicitly test special cases of that codec that would otherwise be extraordinarily rare in the existing random tests. If they are executing against random codecs or even random configurations then they just became useless. So thats why I'm concerned: I see this commit causing these failures, and I know we just experienced a significant loss of test coverage to the index format. We are relying upon tests to *fail* to detect this, but unfortunately 'loss of test coverage' doesn't always trigger a jenkins build. So maybe instead of playing whack-a-mole with jenkins tests failures, we should pay more attention reviewing *all* changes to unit tests where clone() was previously used. The patch is buggy here, and I just want to ensure its taken seriously so that we do not lose coverage > Remove IndexWriterConfig.clone > -- > > Key: LUCENE-5708 > URL: https://issues.apache.org/jira/browse/LUCENE-5708 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Michael McCandless >Assignee: Michael McCandless > Fix For: 4.9, 5.0 > > Attachments: LUCENE-5708.patch, LUCENE-5708.patch, LUCENE-5708.patch > > > We originally added this clone to allow a single IWC to be re-used against > more than one IndexWriter, but I think this is a mis-feature: it adds > complexity to hairy classes (merge policy/scheduler, DW thread pool, etc.), I > think it's buggy today. > I think we should just disallow sharing: you must make a new IWC for a new > IndexWriter. -- This message was sent by Atlassian JIRA (v6.2#6252) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-5708) Remove IndexWriterConfig.clone
[ https://issues.apache.org/jira/browse/LUCENE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14013498#comment-14013498 ] Michael McCandless commented on LUCENE-5708: I agree that the distinction of "randomly test this PF via the generic base test class" and the "specifically test tricky corner cases for this particular PF" (e.g. TestBlockPostingsFormat/2/3.java) is important, and the specific IWC settings for those tests are necessary. I reviewed all the test changes more closely, and found a couple other places that needed to carry over explicit IWC changes after pulling a random IWC (I'll commit shortly). I think this is net/net good vs the clone we had before: it means we are still randomly changing the things the test didn't care about, and fixing the settings that it does. > Remove IndexWriterConfig.clone > -- > > Key: LUCENE-5708 > URL: https://issues.apache.org/jira/browse/LUCENE-5708 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Michael McCandless >Assignee: Michael McCandless > Fix For: 4.9, 5.0 > > Attachments: LUCENE-5708.patch, LUCENE-5708.patch, LUCENE-5708.patch > > > We originally added this clone to allow a single IWC to be re-used against > more than one IndexWriter, but I think this is a mis-feature: it adds > complexity to hairy classes (merge policy/scheduler, DW thread pool, etc.), I > think it's buggy today. > I think we should just disallow sharing: you must make a new IWC for a new > IndexWriter. -- This message was sent by Atlassian JIRA (v6.2#6252) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-5708) Remove IndexWriterConfig.clone
[ https://issues.apache.org/jira/browse/LUCENE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14013506#comment-14013506 ] ASF subversion and git services commented on LUCENE-5708: - Commit 1598543 from [~mikemccand] in branch 'dev/trunk' [ https://svn.apache.org/r1598543 ] LUCENE-5708: fix these tests to also 'mimic' previous IWC.clone > Remove IndexWriterConfig.clone > -- > > Key: LUCENE-5708 > URL: https://issues.apache.org/jira/browse/LUCENE-5708 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Michael McCandless >Assignee: Michael McCandless > Fix For: 4.9, 5.0 > > Attachments: LUCENE-5708.patch, LUCENE-5708.patch, LUCENE-5708.patch > > > We originally added this clone to allow a single IWC to be re-used against > more than one IndexWriter, but I think this is a mis-feature: it adds > complexity to hairy classes (merge policy/scheduler, DW thread pool, etc.), I > think it's buggy today. > I think we should just disallow sharing: you must make a new IWC for a new > IndexWriter. -- This message was sent by Atlassian JIRA (v6.2#6252) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-5708) Remove IndexWriterConfig.clone
[ https://issues.apache.org/jira/browse/LUCENE-5708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14013508#comment-14013508 ] ASF subversion and git services commented on LUCENE-5708: - Commit 1598545 from [~mikemccand] in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1598545 ] LUCENE-5708: fix these tests to also 'mimic' previous IWC.clone > Remove IndexWriterConfig.clone > -- > > Key: LUCENE-5708 > URL: https://issues.apache.org/jira/browse/LUCENE-5708 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Michael McCandless >Assignee: Michael McCandless > Fix For: 4.9, 5.0 > > Attachments: LUCENE-5708.patch, LUCENE-5708.patch, LUCENE-5708.patch > > > We originally added this clone to allow a single IWC to be re-used against > more than one IndexWriter, but I think this is a mis-feature: it adds > complexity to hairy classes (merge policy/scheduler, DW thread pool, etc.), I > think it's buggy today. > I think we should just disallow sharing: you must make a new IWC for a new > IndexWriter. -- This message was sent by Atlassian JIRA (v6.2#6252) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
