You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/05/01 13:54:42 UTC

[GitHub] [lucene-solr] jpountz opened a new pull request #1475: LUCENE-9148: Move the BKD index to its own file.

jpountz opened a new pull request #1475:
URL: https://github.com/apache/lucene-solr/pull/1475


   See https://issues.apache.org/jira/browse/LUCENE-9148.
   
   This is only a work-in-progress for now, I'll finish when #1440 is merged to not duplicate work and introduce conflicts.


----------------------------------------------------------------
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.

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


[GitHub] [lucene-solr] uschindler commented on pull request #1475: LUCENE-9148: Move the BKD index to its own file.

Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #1475:
URL: https://github.com/apache/lucene-solr/pull/1475#issuecomment-640195438


   I think the writer should only be in the test sources like the RWCodec


----------------------------------------------------------------
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.

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


[GitHub] [lucene-solr] iverase commented on a change in pull request #1475: LUCENE-9148: Move the BKD index to its own file.

Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #1475:
URL: https://github.com/apache/lucene-solr/pull/1475#discussion_r436514171



##########
File path: lucene/core/src/java/org/apache/lucene/util/bkd/BKDWriter.java
##########
@@ -627,20 +638,20 @@ assert valueInOrder(valueCount + leafCount,
       assert (lastDocID = docID) >= 0; // only assign when asserts are enabled
     }
 
-    public long finish() throws IOException {
+    public Runnable finish() throws IOException {
       if (leafCount > 0) {
         writeLeafBlock(leafCardinality);
         leafCardinality = 0;
         leafCount = 0;
       }
 
       if (valueCount == 0) {
-        return -1;
+        return null;
       }
 
       pointCount = valueCount;
 
-      long indexFP = out.getFilePointer();
+      long indexFP = indexOut.getFilePointer();

Review comment:
       Not used any more?




----------------------------------------------------------------
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.

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


[GitHub] [lucene-solr] iverase commented on pull request #1475: LUCENE-9148: Move the BKD index to its own file.

Posted by GitBox <gi...@apache.org>.
iverase commented on pull request #1475:
URL: https://github.com/apache/lucene-solr/pull/1475#issuecomment-640198022


   I missed that, it is on the test sources. Thanks for the clarification!


----------------------------------------------------------------
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.

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


[GitHub] [lucene-solr] jpountz merged pull request #1475: LUCENE-9148: Move the BKD index to its own file.

Posted by GitBox <gi...@apache.org>.
jpountz merged pull request #1475:
URL: https://github.com/apache/lucene-solr/pull/1475


   


----------------------------------------------------------------
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.

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


[GitHub] [lucene-solr] iverase commented on a change in pull request #1475: LUCENE-9148: Move the BKD index to its own file.

Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #1475:
URL: https://github.com/apache/lucene-solr/pull/1475#discussion_r436513642



##########
File path: lucene/core/src/java/org/apache/lucene/codecs/lucene86/Lucene86PointsWriter.java
##########
@@ -0,0 +1,262 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.codecs.lucene86;
+
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.lucene.codecs.CodecUtil;
+import org.apache.lucene.codecs.MutablePointValues;
+import org.apache.lucene.codecs.PointsReader;
+import org.apache.lucene.codecs.PointsWriter;
+import org.apache.lucene.index.FieldInfo;
+import org.apache.lucene.index.FieldInfos;
+import org.apache.lucene.index.IndexFileNames;
+import org.apache.lucene.index.MergeState;
+import org.apache.lucene.index.PointValues;
+import org.apache.lucene.index.PointValues.IntersectVisitor;
+import org.apache.lucene.index.PointValues.Relation;
+import org.apache.lucene.index.SegmentWriteState;
+import org.apache.lucene.store.IndexOutput;
+import org.apache.lucene.util.IOUtils;
+import org.apache.lucene.util.bkd.BKDReader;
+import org.apache.lucene.util.bkd.BKDWriter;
+
+/** Writes dimensional values */
+public class Lucene86PointsWriter extends PointsWriter implements Closeable {
+
+  /** Outputs used to write the BKD tree data files. */
+  protected final IndexOutput metaOut, indexOut, dataOut;
+
+  final SegmentWriteState writeState;
+  final int maxPointsInLeafNode;
+  final double maxMBSortInHeap;
+  private boolean finished;
+
+  /** Full constructor */
+  public Lucene86PointsWriter(SegmentWriteState writeState, int maxPointsInLeafNode, double maxMBSortInHeap) throws IOException {
+    assert writeState.fieldInfos.hasPointValues();
+    this.writeState = writeState;
+    this.maxPointsInLeafNode = maxPointsInLeafNode;
+    this.maxMBSortInHeap = maxMBSortInHeap;
+    String dataFileName = IndexFileNames.segmentFileName(writeState.segmentInfo.name,
+                                                         writeState.segmentSuffix,
+                                                         Lucene86PointsFormat.DATA_EXTENSION);
+    dataOut = writeState.directory.createOutput(dataFileName, writeState.context);
+    boolean success = false;
+    try {
+      CodecUtil.writeIndexHeader(dataOut,
+                                 Lucene86PointsFormat.DATA_CODEC_NAME,
+                                 Lucene86PointsFormat.VERSION_CURRENT,
+                                 writeState.segmentInfo.getId(),
+                                 writeState.segmentSuffix);
+
+      String metaFileName = IndexFileNames.segmentFileName(writeState.segmentInfo.name,
+          writeState.segmentSuffix,
+          Lucene86PointsFormat.META_EXTENSION);
+      metaOut = writeState.directory.createOutput(metaFileName, writeState.context);
+      CodecUtil.writeIndexHeader(metaOut,
+          Lucene86PointsFormat.META_CODEC_NAME,
+          Lucene86PointsFormat.VERSION_CURRENT,
+          writeState.segmentInfo.getId(),
+          writeState.segmentSuffix);
+
+      String indexFileName = IndexFileNames.segmentFileName(writeState.segmentInfo.name,
+          writeState.segmentSuffix,
+          Lucene86PointsFormat.INDEX_EXTENSION);
+      indexOut = writeState.directory.createOutput(indexFileName, writeState.context);
+      CodecUtil.writeIndexHeader(indexOut,
+          Lucene86PointsFormat.INDEX_CODEC_NAME,
+          Lucene86PointsFormat.VERSION_CURRENT,
+          writeState.segmentInfo.getId(),
+          writeState.segmentSuffix);
+
+      success = true;
+    } finally {
+      if (success == false) {
+        IOUtils.closeWhileHandlingException(this);
+      }
+    }
+  }
+
+  /** Uses the defaults values for {@code maxPointsInLeafNode} (1024) and {@code maxMBSortInHeap} (16.0) */
+  public Lucene86PointsWriter(SegmentWriteState writeState) throws IOException {
+    this(writeState, BKDWriter.DEFAULT_MAX_POINTS_IN_LEAF_NODE, BKDWriter.DEFAULT_MAX_MB_SORT_IN_HEAP);
+  }
+
+  @Override
+  public void writeField(FieldInfo fieldInfo, PointsReader reader) throws IOException {
+
+    PointValues values = reader.getValues(fieldInfo.name);
+
+    try (BKDWriter writer = new BKDWriter(writeState.segmentInfo.maxDoc(),
+                                          writeState.directory,
+                                          writeState.segmentInfo.name,
+                                          fieldInfo.getPointDimensionCount(),
+                                          fieldInfo.getPointIndexDimensionCount(),
+                                          fieldInfo.getPointNumBytes(),
+                                          maxPointsInLeafNode,
+                                          maxMBSortInHeap,
+                                          values.size())) {
+
+      if (values instanceof MutablePointValues) {
+        Runnable finalizer = writer.writeField(metaOut, indexOut, dataOut, fieldInfo.name, (MutablePointValues) values);
+        if (finalizer != null) {
+          metaOut.writeInt(fieldInfo.number);
+          finalizer.run();
+        }
+        return;
+      }
+
+      values.intersect(new IntersectVisitor() {
+          @Override
+          public void visit(int docID) {
+            throw new IllegalStateException();
+          }
+
+          public void visit(int docID, byte[] packedValue) throws IOException {
+            writer.add(packedValue, docID);
+          }
+
+          @Override
+          public Relation compare(byte[] minPackedValue, byte[] maxPackedValue) {
+            return Relation.CELL_CROSSES_QUERY;
+          }
+        });
+
+      // We could have 0 points on merge since all docs with dimensional fields may be deleted:
+      if (writer.getPointCount() > 0) {

Review comment:
       I wonder if we should use the same structure as in the other places by returning here as well a runnable for consistency?




----------------------------------------------------------------
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.

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


[GitHub] [lucene-solr] iverase commented on pull request #1475: LUCENE-9148: Move the BKD index to its own file.

Posted by GitBox <gi...@apache.org>.
iverase commented on pull request #1475:
URL: https://github.com/apache/lucene-solr/pull/1475#issuecomment-640178481


   Once we move a codec to backwards codecs, it can only be used for reading and still we are moving the `Lucene60PointsWriter`  to that package. Is that necessary?  


----------------------------------------------------------------
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.

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