[GitHub] [lucene] zhaih commented on a diff in pull request #11815: Support deletions in rearrange (#11814)

[GitBox <gi...@apache.org>]

zhaih commented on code in PR #11815:
URL: https://github.com/apache/lucene/pull/11815#discussion_r979581760


##########
lucene/misc/src/java/org/apache/lucene/misc/index/IndexRearranger.java:
##########
@@ -175,5 +202,7 @@ public CacheHelper getReaderCacheHelper() {
   /** Select document within a CodecReader */
   public interface DocumentSelector {
     BitSet getFilteredLiveDocs(CodecReader reader) throws IOException;
+
+    boolean isDeleted(LeafReader reader, int idx) throws IOException;

Review Comment:
   I think it is confusing to put this here:
   1. The first method `getFilteredLiveDocs` is basically from "original" index, generating a set of documents that will be pulled into the "new" index. But the new method is basically from "new" index, query whether a document is deleted. You see that they basically are supposed to be applied to different index and it is quite confusing.
   2. Given that this method are supposed to be applied to the "new" index, I would assume the creator will have some information on which documents are supposed to be deleted beforehand, like a set of ids that are deleted in the "origin" index. Then why don't we put this deletion logic after the rearrange has done? I don't see any advantage of putting the logic here. Also now the `writer.addIndexes` are subject to a lock which means we're slowing things down quite a lot.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org