[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[ https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15883608#comment-15883608 ] Michael McCandless commented on LUCENE-7707: OK I committed that last patch. > Only assign ScoreDoc#shardIndex if it was already assigned to non default > (-1) value > > > Key: LUCENE-7707 > URL: https://issues.apache.org/jira/browse/LUCENE-7707 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Simon Willnauer >Assignee: Simon Willnauer > Fix For: master (7.0), 6.5.0 > > Attachments: LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch, > LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch, > LUCENE-7707.patch > > > When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex > value. The assumption that is made here is that all shard results are merges > at once which is not necessarily the case. If for instance incremental merge > phases are applied the shard index doesn't correspond to the index in the > outer TopDocs array. To make this a backwards compatible but yet > non-controversial change we could change the internals of TopDocs#merge to > only assign this value unless it's not been assigned before to a non-default > (-1) value to allow multiple or sparse top docs merging. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[ https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15883605#comment-15883605 ] ASF subversion and git services commented on LUCENE-7707: - Commit 2e56c0e50564c8feeeb2831dd36cff1e9b23a00f in lucene-solr's branch refs/heads/master from Mike McCandless [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2e56c0e ] LUCENE-7707: add explicit boolean to TopDocs.merge to govern whether incoming or implicit shard index should be used > Only assign ScoreDoc#shardIndex if it was already assigned to non default > (-1) value > > > Key: LUCENE-7707 > URL: https://issues.apache.org/jira/browse/LUCENE-7707 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Simon Willnauer >Assignee: Simon Willnauer > Fix For: master (7.0), 6.5.0 > > Attachments: LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch, > LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch, > LUCENE-7707.patch > > > When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex > value. The assumption that is made here is that all shard results are merges > at once which is not necessarily the case. If for instance incremental merge > phases are applied the shard index doesn't correspond to the index in the > outer TopDocs array. To make this a backwards compatible but yet > non-controversial change we could change the internals of TopDocs#merge to > only assign this value unless it's not been assigned before to a non-default > (-1) value to allow multiple or sparse top docs merging. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[ https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15883599#comment-15883599 ] ASF subversion and git services commented on LUCENE-7707: - Commit d00c5cae2b80941bbe71c091d42659e0c504b5ec in lucene-solr's branch refs/heads/branch_6x from Mike McCandless [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=d00c5ca ] LUCENE-7707: add explicit boolean to TopDocs.merge to govern whether incoming or implicit shard index should be used > Only assign ScoreDoc#shardIndex if it was already assigned to non default > (-1) value > > > Key: LUCENE-7707 > URL: https://issues.apache.org/jira/browse/LUCENE-7707 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Simon Willnauer >Assignee: Simon Willnauer > Fix For: master (7.0), 6.5.0 > > Attachments: LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch, > LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch, > LUCENE-7707.patch > > > When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex > value. The assumption that is made here is that all shard results are merges > at once which is not necessarily the case. If for instance incremental merge > phases are applied the shard index doesn't correspond to the index in the > outer TopDocs array. To make this a backwards compatible but yet > non-controversial change we could change the internals of TopDocs#merge to > only assign this value unless it's not been assigned before to a non-default > (-1) value to allow multiple or sparse top docs merging. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[
https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15882798#comment-15882798
]
Adrien Grand commented on LUCENE-7707:
--
Let's make sure that the shard index is not -1 if {{setShardIndex}} is false?
Otherwise + 1.
> Only assign ScoreDoc#shardIndex if it was already assigned to non default
> (-1) value
>
>
> Key: LUCENE-7707
> URL: https://issues.apache.org/jira/browse/LUCENE-7707
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Simon Willnauer
>Assignee: Simon Willnauer
> Fix For: master (7.0), 6.5.0
>
> Attachments: LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch,
> LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch
>
>
> When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex
> value. The assumption that is made here is that all shard results are merges
> at once which is not necessarily the case. If for instance incremental merge
> phases are applied the shard index doesn't correspond to the index in the
> outer TopDocs array. To make this a backwards compatible but yet
> non-controversial change we could change the internals of TopDocs#merge to
> only assign this value unless it's not been assigned before to a non-default
> (-1) value to allow multiple or sparse top docs merging.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[
https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15882531#comment-15882531
]
Michael McCandless commented on LUCENE-7707:
Alas, I think we do need to add an explicit boolean to the public API ... this
test failure repros for me:
{noformat}
[junit4] Suite: org.apache.lucene.search.TestShardSearching
[junit4] 2> NOTE: reproduce with: ant test -Dtestcase=TestShardSearching
-Dtests.method=testSimple -Dtests.seed=2D10A476239970A9 -Dtests.slow=true
-Dtests.locale=en-NZ -Dtests.timezone=Navajo -Dtests.asserts=true
-Dtests.file.encoding=UTF8
[junit4] FAILURE 0.64s J2 | TestShardSearching.testSimple <<<
[junit4]> Throwable #1: java.lang.AssertionError
[junit4]>at
__randomizedtesting.SeedInfo.seed([2D10A476239970A9:15A38088046AA478]:0)
[junit4]>at
org.apache.lucene.search.TopDocs.tieBreakLessThan(TopDocs.java:104)
[junit4]>at
org.apache.lucene.search.TopDocs$MergeSortQueue.lessThan(TopDocs.java:196)
[junit4]>at
org.apache.lucene.search.TopDocs$MergeSortQueue.lessThan(TopDocs.java:138)
[junit4]>at
org.apache.lucene.util.PriorityQueue.upHeap(PriorityQueue.java:263)
[junit4]>at
org.apache.lucene.util.PriorityQueue.add(PriorityQueue.java:140)
[junit4]>at
org.apache.lucene.search.TopDocs.mergeAux(TopDocs.java:283)
[junit4]>at
org.apache.lucene.search.TopDocs.merge(TopDocs.java:248)
[junit4]>at
org.apache.lucene.search.TopDocs.merge(TopDocs.java:232)
[junit4]>at
org.apache.lucene.search.ShardSearchingTestBase$NodeState$ShardIndexSearcher.search(ShardSearchingTestBase.java:440)
[junit4]>at
org.apache.lucene.search.TestShardSearching.assertSame(TestShardSearching.java:313)
[junit4]>at
org.apache.lucene.search.TestShardSearching.testSimple(TestShardSearching.java:236)
[junit4]>at java.lang.Thread.run(Thread.java:745)
{noformat}
It happens because IndexSearcher will sometimes (when using an executor) set
the shardIndex of the TopDocs it returns to the caller, but it should not (so
that the caller can then later do their own merging). Likewise, grouping,
drill sideways ...
I'll work on a patch to make it explicit...
> Only assign ScoreDoc#shardIndex if it was already assigned to non default
> (-1) value
>
>
> Key: LUCENE-7707
> URL: https://issues.apache.org/jira/browse/LUCENE-7707
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Simon Willnauer
>Assignee: Simon Willnauer
> Fix For: master (7.0), 6.5.0
>
> Attachments: LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch,
> LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch
>
>
> When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex
> value. The assumption that is made here is that all shard results are merges
> at once which is not necessarily the case. If for instance incremental merge
> phases are applied the shard index doesn't correspond to the index in the
> outer TopDocs array. To make this a backwards compatible but yet
> non-controversial change we could change the internals of TopDocs#merge to
> only assign this value unless it's not been assigned before to a non-default
> (-1) value to allow multiple or sparse top docs merging.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[ https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15882440#comment-15882440 ] ASF subversion and git services commented on LUCENE-7707: - Commit 7f904a2bccad38929ced27b5ccd81fbc6df5d8ff in lucene-solr's branch refs/heads/branch_6x from [~simonw] [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7f904a2 ] LUCENE-7707: Use predefined shard index when mergeing top docs if present. This allows to use TopDoc#merge to merge shard responses incrementally instead of once all shard responses are present. > Only assign ScoreDoc#shardIndex if it was already assigned to non default > (-1) value > > > Key: LUCENE-7707 > URL: https://issues.apache.org/jira/browse/LUCENE-7707 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Simon Willnauer > Fix For: master (7.0), 6.5.0 > > Attachments: LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch, > LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch > > > When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex > value. The assumption that is made here is that all shard results are merges > at once which is not necessarily the case. If for instance incremental merge > phases are applied the shard index doesn't correspond to the index in the > outer TopDocs array. To make this a backwards compatible but yet > non-controversial change we could change the internals of TopDocs#merge to > only assign this value unless it's not been assigned before to a non-default > (-1) value to allow multiple or sparse top docs merging. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[ https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15882434#comment-15882434 ] ASF subversion and git services commented on LUCENE-7707: - Commit 5eeb8136f34fc7b3e2157982e5fa42da7f115d76 in lucene-solr's branch refs/heads/master from [~simonw] [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=5eeb813 ] LUCENE-7707: Use predefined shard index when mergeing top docs if present. This allows to use TopDoc#merge to merge shard responses incrementally instead of once all shard responses are present. > Only assign ScoreDoc#shardIndex if it was already assigned to non default > (-1) value > > > Key: LUCENE-7707 > URL: https://issues.apache.org/jira/browse/LUCENE-7707 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Simon Willnauer > Fix For: master (7.0), 6.5.0 > > Attachments: LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch, > LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch > > > When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex > value. The assumption that is made here is that all shard results are merges > at once which is not necessarily the case. If for instance incremental merge > phases are applied the shard index doesn't correspond to the index in the > outer TopDocs array. To make this a backwards compatible but yet > non-controversial change we could change the internals of TopDocs#merge to > only assign this value unless it's not been assigned before to a non-default > (-1) value to allow multiple or sparse top docs merging. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[ https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15882185#comment-15882185 ] Adrien Grand commented on LUCENE-7707: -- Yes, thanks for adding this validation. +1 to the patch > Only assign ScoreDoc#shardIndex if it was already assigned to non default > (-1) value > > > Key: LUCENE-7707 > URL: https://issues.apache.org/jira/browse/LUCENE-7707 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Simon Willnauer > Fix For: master (7.0), 6.5.0 > > Attachments: LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch, > LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch > > > When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex > value. The assumption that is made here is that all shard results are merges > at once which is not necessarily the case. If for instance incremental merge > phases are applied the shard index doesn't correspond to the index in the > outer TopDocs array. To make this a backwards compatible but yet > non-controversial change we could change the internals of TopDocs#merge to > only assign this value unless it's not been assigned before to a non-default > (-1) value to allow multiple or sparse top docs merging. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[ https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15882136#comment-15882136 ] Simon Willnauer commented on LUCENE-7707: - [~jpountz] are your concerns addressed with the latest patch? > Only assign ScoreDoc#shardIndex if it was already assigned to non default > (-1) value > > > Key: LUCENE-7707 > URL: https://issues.apache.org/jira/browse/LUCENE-7707 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Simon Willnauer > Fix For: master (7.0), 6.5.0 > > Attachments: LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch, > LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch > > > When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex > value. The assumption that is made here is that all shard results are merges > at once which is not necessarily the case. If for instance incremental merge > phases are applied the shard index doesn't correspond to the index in the > outer TopDocs array. To make this a backwards compatible but yet > non-controversial change we could change the internals of TopDocs#merge to > only assign this value unless it's not been assigned before to a non-default > (-1) value to allow multiple or sparse top docs merging. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[ https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15881181#comment-15881181 ] Michael McCandless commented on LUCENE-7707: +1, thanks [~simonw]. > Only assign ScoreDoc#shardIndex if it was already assigned to non default > (-1) value > > > Key: LUCENE-7707 > URL: https://issues.apache.org/jira/browse/LUCENE-7707 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Simon Willnauer > Fix For: master (7.0), 6.5.0 > > Attachments: LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch, > LUCENE-7707.patch, LUCENE-7707.patch > > > When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex > value. The assumption that is made here is that all shard results are merges > at once which is not necessarily the case. If for instance incremental merge > phases are applied the shard index doesn't correspond to the index in the > outer TopDocs array. To make this a backwards compatible but yet > non-controversial change we could change the internals of TopDocs#merge to > only assign this value unless it's not been assigned before to a non-default > (-1) value to allow multiple or sparse top docs merging. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[ https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15881132#comment-15881132 ] Michael McCandless commented on LUCENE-7707: +1, looks awesome! Maybe update the javadocs to explain that we will either fill in the shardIndex or will not but never both. > Only assign ScoreDoc#shardIndex if it was already assigned to non default > (-1) value > > > Key: LUCENE-7707 > URL: https://issues.apache.org/jira/browse/LUCENE-7707 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Simon Willnauer > Fix For: master (7.0), 6.5.0 > > Attachments: LUCENE-7707.patch, LUCENE-7707.patch, LUCENE-7707.patch, > LUCENE-7707.patch > > > When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex > value. The assumption that is made here is that all shard results are merges > at once which is not necessarily the case. If for instance incremental merge > phases are applied the shard index doesn't correspond to the index in the > outer TopDocs array. To make this a backwards compatible but yet > non-controversial change we could change the internals of TopDocs#merge to > only assign this value unless it's not been assigned before to a non-default > (-1) value to allow multiple or sparse top docs merging. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[ https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15881076#comment-15881076 ] Simon Willnauer commented on LUCENE-7707: - bq. Maybe we could require that either all incoming shardIndex are undefined, or all are set, but you are not allowed to mix? I think this is what we should ultimately do. I don't see a different way than peaking at the at the TopDocs so see if it's preset and then executed based on that. I can certainly add assertions... > Only assign ScoreDoc#shardIndex if it was already assigned to non default > (-1) value > > > Key: LUCENE-7707 > URL: https://issues.apache.org/jira/browse/LUCENE-7707 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Simon Willnauer > Fix For: master (7.0), 6.5.0 > > Attachments: LUCENE-7707.patch, LUCENE-7707.patch > > > When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex > value. The assumption that is made here is that all shard results are merges > at once which is not necessarily the case. If for instance incremental merge > phases are applied the shard index doesn't correspond to the index in the > outer TopDocs array. To make this a backwards compatible but yet > non-controversial change we could change the internals of TopDocs#merge to > only assign this value unless it's not been assigned before to a non-default > (-1) value to allow multiple or sparse top docs merging. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[
https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15880862#comment-15880862
]
Michael McCandless commented on LUCENE-7707:
Maybe we could require that either all incoming {{shardIndex}} are undefined,
or all are set, but you are not allowed to mix?
> Only assign ScoreDoc#shardIndex if it was already assigned to non default
> (-1) value
>
>
> Key: LUCENE-7707
> URL: https://issues.apache.org/jira/browse/LUCENE-7707
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Simon Willnauer
> Fix For: master (7.0), 6.5.0
>
> Attachments: LUCENE-7707.patch, LUCENE-7707.patch
>
>
> When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex
> value. The assumption that is made here is that all shard results are merges
> at once which is not necessarily the case. If for instance incremental merge
> phases are applied the shard index doesn't correspond to the index in the
> outer TopDocs array. To make this a backwards compatible but yet
> non-controversial change we could change the internals of TopDocs#merge to
> only assign this value unless it's not been assigned before to a non-default
> (-1) value to allow multiple or sparse top docs merging.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[ https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15880859#comment-15880859 ] Jim Ferenczi commented on LUCENE-7707: -- bq. Plus I think it's very unlikely someone today is pre-setting the shardIndex (off of it's default -1 value) and then relying on TopDocs.merge Good point. +1 to the patch too, there's nothing to break here ;) > Only assign ScoreDoc#shardIndex if it was already assigned to non default > (-1) value > > > Key: LUCENE-7707 > URL: https://issues.apache.org/jira/browse/LUCENE-7707 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Simon Willnauer > Fix For: master (7.0), 6.5.0 > > Attachments: LUCENE-7707.patch, LUCENE-7707.patch > > > When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex > value. The assumption that is made here is that all shard results are merges > at once which is not necessarily the case. If for instance incremental merge > phases are applied the shard index doesn't correspond to the index in the > outer TopDocs array. To make this a backwards compatible but yet > non-controversial change we could change the internals of TopDocs#merge to > only assign this value unless it's not been assigned before to a non-default > (-1) value to allow multiple or sparse top docs merging. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[ https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15880854#comment-15880854 ] Adrien Grand commented on LUCENE-7707: -- I don't like the fact that if you mix top docs that have the shard index set and other instances that have it undefined, then we could end up assigning a shard id that is already in use. Is there a way we can avoid that? > Only assign ScoreDoc#shardIndex if it was already assigned to non default > (-1) value > > > Key: LUCENE-7707 > URL: https://issues.apache.org/jira/browse/LUCENE-7707 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Simon Willnauer > Fix For: master (7.0), 6.5.0 > > Attachments: LUCENE-7707.patch, LUCENE-7707.patch > > > When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex > value. The assumption that is made here is that all shard results are merges > at once which is not necessarily the case. If for instance incremental merge > phases are applied the shard index doesn't correspond to the index in the > outer TopDocs array. To make this a backwards compatible but yet > non-controversial change we could change the internals of TopDocs#merge to > only assign this value unless it's not been assigned before to a non-default > (-1) value to allow multiple or sparse top docs merging. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[ https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15880838#comment-15880838 ] Michael McCandless commented on LUCENE-7707: +1 to the patch. bq. I personally would like that but Michael McCandless had some issues with this? Yeah, I'd prefer not to add a boolean argument: that's allowing a temporary back compat issue to have a permanent impact on our APIs. Our APIs should be designed for future usage. Plus I think it's very unlikely someone today is pre-setting the shardIndex (off of it's default -1 value) and then relying on TopDocs.merge to overwrite it. I think the patch is sufficient back compat behavior w/o a compromised API change. > Only assign ScoreDoc#shardIndex if it was already assigned to non default > (-1) value > > > Key: LUCENE-7707 > URL: https://issues.apache.org/jira/browse/LUCENE-7707 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Simon Willnauer > Fix For: master (7.0), 6.5.0 > > Attachments: LUCENE-7707.patch, LUCENE-7707.patch > > > When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex > value. The assumption that is made here is that all shard results are merges > at once which is not necessarily the case. If for instance incremental merge > phases are applied the shard index doesn't correspond to the index in the > outer TopDocs array. To make this a backwards compatible but yet > non-controversial change we could change the internals of TopDocs#merge to > only assign this value unless it's not been assigned before to a non-default > (-1) value to allow multiple or sparse top docs merging. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[ https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15880767#comment-15880767 ] Simon Willnauer commented on LUCENE-7707: - I personally think making this solely dependent on a boolean would be best IMO. It would be an additional overload of the methods that explicitly turns on that shardIndex is set on the ScoreDoc and we don't need to do as much conditionals in the tie-breaking. I personally would like that but [~mikemccand] had some issues with this? > Only assign ScoreDoc#shardIndex if it was already assigned to non default > (-1) value > > > Key: LUCENE-7707 > URL: https://issues.apache.org/jira/browse/LUCENE-7707 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Simon Willnauer > Fix For: master (7.0), 6.5.0 > > Attachments: LUCENE-7707.patch, LUCENE-7707.patch > > > When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex > value. The assumption that is made here is that all shard results are merges > at once which is not necessarily the case. If for instance incremental merge > phases are applied the shard index doesn't correspond to the index in the > outer TopDocs array. To make this a backwards compatible but yet > non-controversial change we could change the internals of TopDocs#merge to > only assign this value unless it's not been assigned before to a non-default > (-1) value to allow multiple or sparse top docs merging. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-7707) Only assign ScoreDoc#shardIndex if it was already assigned to non default (-1) value
[ https://issues.apache.org/jira/browse/LUCENE-7707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15880708#comment-15880708 ] Jim Ferenczi commented on LUCENE-7707: -- +1, this will make the merge more flexible. If we really want to be sure that it does not break the BWC maybe it can be an option of the merge function ? A simple boolean overrideShardIndex with a default value of false ? > Only assign ScoreDoc#shardIndex if it was already assigned to non default > (-1) value > > > Key: LUCENE-7707 > URL: https://issues.apache.org/jira/browse/LUCENE-7707 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Simon Willnauer > Fix For: master (7.0), 6.5.0 > > Attachments: LUCENE-7707.patch, LUCENE-7707.patch > > > When you use TopDocs.merge today it always overrides the ScoreDoc#shardIndex > value. The assumption that is made here is that all shard results are merges > at once which is not necessarily the case. If for instance incremental merge > phases are applied the shard index doesn't correspond to the index in the > outer TopDocs array. To make this a backwards compatible but yet > non-controversial change we could change the internals of TopDocs#merge to > only assign this value unless it's not been assigned before to a non-default > (-1) value to allow multiple or sparse top docs merging. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
