You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/03/02 15:28:23 UTC
[GitHub] [pinot] richardstartin opened a new pull request #8279: Make realtime indexes pluggable
richardstartin opened a new pull request #8279:
URL: https://github.com/apache/pinot/pull/8279
This is PR simply moves static realtime index provisioning logic out of `MutableSegmentImpl`'s constructor and into a default implementation of an SPI which can overridden.
The changes depend on a couple of package moves (#8277, #8278) which I would like to get merged in before rebasing this PR to reduce the surface area of this one.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin merged pull request #8279: Make realtime indexes pluggable
Posted by GitBox <gi...@apache.org>.
richardstartin merged pull request #8279:
URL: https://github.com/apache/pinot/pull/8279
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #8279: Make realtime indexes pluggable
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8279:
URL: https://github.com/apache/pinot/pull/8279#discussion_r819807948
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentImpl.java
##########
@@ -242,6 +235,10 @@ public long getLatestIngestionTimestamp() {
// Initialize for each column
for (FieldSpec fieldSpec : _physicalFieldSpecs) {
String column = fieldSpec.getName();
+ boolean isRaw = isNoDictionaryColumn(noDictionaryColumns, invertedIndexColumns, fieldSpec, column);
Review comment:
Changed it to `isDictionary` and inverted its truth value here.
##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/mutable/provider/MutableForwardIndexProvider.java
##########
@@ -0,0 +1,26 @@
+/**
+ * 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.pinot.segment.spi.index.mutable.provider;
+
+import org.apache.pinot.segment.spi.index.mutable.MutableForwardIndex;
+
+
+public interface MutableForwardIndexProvider {
+ MutableForwardIndex newIndex(MutableIndexContext.Forward context);
Review comment:
OK
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] sajjad-moradi commented on a change in pull request #8279: Make realtime indexes pluggable
Posted by GitBox <gi...@apache.org>.
sajjad-moradi commented on a change in pull request #8279:
URL: https://github.com/apache/pinot/pull/8279#discussion_r819811419
##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/mutable/provider/MutableIndexContext.java
##########
@@ -0,0 +1,249 @@
+/**
+ * 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.pinot.segment.spi.index.mutable.provider;
+
+import java.util.Objects;
+import org.apache.pinot.segment.spi.memory.PinotDataBufferMemoryManager;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public interface MutableIndexContext {
+ PinotDataBufferMemoryManager getMemoryManager();
+
+ FieldSpec getFieldSpec();
+
+ String getSegmentName();
+
+ boolean hasDictionary();
+
+ int getCapacity();
+
+ boolean isOffHeap();
+
+ static Builder builder() {
+ return new Builder();
+ }
+
+ class Builder {
+ private FieldSpec _fieldSpec;
+ private String _segmentName;
+ private boolean _hasDictionary = true;
+ private boolean _offHeap = true;
+ private int _capacity;
+ private PinotDataBufferMemoryManager _memoryManager;
+
+ public Builder withMemoryManager(PinotDataBufferMemoryManager memoryManager) {
+ _memoryManager = memoryManager;
+ return this;
+ }
+
+ public Builder withFieldSpec(FieldSpec fieldSpec) {
+ _fieldSpec = fieldSpec;
+ return this;
+ }
+
+ public Builder withSegmentName(String segmentName) {
+ _segmentName = segmentName;
+ return this;
+ }
+
+ public Builder withDictionary(boolean hasDictionary) {
+ _hasDictionary = hasDictionary;
+ return this;
+ }
+
+ public Builder offHeap(boolean offHeap) {
+ _offHeap = offHeap;
+ return this;
+ }
+
+ public Builder withCapacity(int capacity) {
+ _capacity = capacity;
+ return this;
+ }
+
+ public Common build() {
+ return new Common(Objects.requireNonNull(_fieldSpec), _hasDictionary, Objects.requireNonNull(_segmentName),
+ Objects.requireNonNull(_memoryManager), _capacity, _offHeap);
+ }
+ }
+
+ final class Common implements MutableIndexContext {
+ private final int _capacity;
+ private final FieldSpec _fieldSpec;
+ private final boolean _hasDictionary;
+ private final boolean _offHeap;
+ private final String _segmentName;
+ private final PinotDataBufferMemoryManager _memoryManager;
+
+ public Common(FieldSpec fieldSpec, boolean hasDictionary, String segmentName,
+ PinotDataBufferMemoryManager memoryManager, int capacity, boolean offHeap) {
+ _fieldSpec = fieldSpec;
+ _hasDictionary = hasDictionary;
+ _segmentName = segmentName;
+ _memoryManager = memoryManager;
+ _capacity = capacity;
+ _offHeap = offHeap;
+ }
+
+ @Override
+ public PinotDataBufferMemoryManager getMemoryManager() {
+ return _memoryManager;
+ }
+
+ @Override
+ public String getSegmentName() {
+ return _segmentName;
+ }
+
+ @Override
+ public FieldSpec getFieldSpec() {
+ return _fieldSpec;
+ }
+
+ @Override
+ public boolean hasDictionary() {
+ return _hasDictionary;
+ }
+
+ @Override
+ public int getCapacity() {
+ return _capacity;
+ }
+
+ @Override
+ public boolean isOffHeap() {
+ return _offHeap;
+ }
+
+ public Dictionary forDictionary(int estimatedColSize, int estimatedCardinality) {
+ return new Dictionary(this, estimatedColSize, estimatedCardinality);
+ }
+
+ public Forward forForwardIndex(int avgNumMultiValues) {
+ return new Forward(this, avgNumMultiValues);
+ }
+
+ public Inverted forInvertedIndex() {
+ return new Inverted(this);
+ }
+
+ public Json forJsonIndex() {
+ return new Json(this);
+ }
+
+ public Text forTextIndex() {
+ return new Text(this);
+ }
+ }
+
+ class Wrapper implements MutableIndexContext {
Review comment:
That makes sense. Now the question is why we need Common class! MutableIndexContext can be a class not an interface. In places where it's being used, it's a little bit confusing to instantiate MutableIndexContext.Common while we could simply instantiate MutableIndexCommon.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #8279: Make realtime indexes pluggable
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8279:
URL: https://github.com/apache/pinot/pull/8279#discussion_r819797971
##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/mutable/provider/MutableIndexContext.java
##########
@@ -0,0 +1,249 @@
+/**
+ * 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.pinot.segment.spi.index.mutable.provider;
+
+import java.util.Objects;
+import org.apache.pinot.segment.spi.memory.PinotDataBufferMemoryManager;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public interface MutableIndexContext {
+ PinotDataBufferMemoryManager getMemoryManager();
+
+ FieldSpec getFieldSpec();
+
+ String getSegmentName();
+
+ boolean hasDictionary();
+
+ int getCapacity();
+
+ boolean isOffHeap();
+
+ static Builder builder() {
+ return new Builder();
+ }
+
+ class Builder {
+ private FieldSpec _fieldSpec;
+ private String _segmentName;
+ private boolean _hasDictionary = true;
+ private boolean _offHeap = true;
+ private int _capacity;
+ private PinotDataBufferMemoryManager _memoryManager;
+
+ public Builder withMemoryManager(PinotDataBufferMemoryManager memoryManager) {
+ _memoryManager = memoryManager;
+ return this;
+ }
+
+ public Builder withFieldSpec(FieldSpec fieldSpec) {
+ _fieldSpec = fieldSpec;
+ return this;
+ }
+
+ public Builder withSegmentName(String segmentName) {
+ _segmentName = segmentName;
+ return this;
+ }
+
+ public Builder withDictionary(boolean hasDictionary) {
+ _hasDictionary = hasDictionary;
+ return this;
+ }
+
+ public Builder offHeap(boolean offHeap) {
+ _offHeap = offHeap;
+ return this;
+ }
+
+ public Builder withCapacity(int capacity) {
+ _capacity = capacity;
+ return this;
+ }
+
+ public Common build() {
+ return new Common(Objects.requireNonNull(_fieldSpec), _hasDictionary, Objects.requireNonNull(_segmentName),
+ Objects.requireNonNull(_memoryManager), _capacity, _offHeap);
+ }
+ }
+
+ final class Common implements MutableIndexContext {
+ private final int _capacity;
+ private final FieldSpec _fieldSpec;
+ private final boolean _hasDictionary;
+ private final boolean _offHeap;
+ private final String _segmentName;
+ private final PinotDataBufferMemoryManager _memoryManager;
+
+ public Common(FieldSpec fieldSpec, boolean hasDictionary, String segmentName,
+ PinotDataBufferMemoryManager memoryManager, int capacity, boolean offHeap) {
+ _fieldSpec = fieldSpec;
+ _hasDictionary = hasDictionary;
+ _segmentName = segmentName;
+ _memoryManager = memoryManager;
+ _capacity = capacity;
+ _offHeap = offHeap;
+ }
+
+ @Override
+ public PinotDataBufferMemoryManager getMemoryManager() {
+ return _memoryManager;
+ }
+
+ @Override
+ public String getSegmentName() {
+ return _segmentName;
+ }
+
+ @Override
+ public FieldSpec getFieldSpec() {
+ return _fieldSpec;
+ }
+
+ @Override
+ public boolean hasDictionary() {
+ return _hasDictionary;
+ }
+
+ @Override
+ public int getCapacity() {
+ return _capacity;
+ }
+
+ @Override
+ public boolean isOffHeap() {
+ return _offHeap;
+ }
+
+ public Dictionary forDictionary(int estimatedColSize, int estimatedCardinality) {
+ return new Dictionary(this, estimatedColSize, estimatedCardinality);
+ }
+
+ public Forward forForwardIndex(int avgNumMultiValues) {
+ return new Forward(this, avgNumMultiValues);
+ }
+
+ public Inverted forInvertedIndex() {
+ return new Inverted(this);
+ }
+
+ public Json forJsonIndex() {
+ return new Json(this);
+ }
+
+ public Text forTextIndex() {
+ return new Text(this);
+ }
+ }
+
+ class Wrapper implements MutableIndexContext {
Review comment:
They could, but then they also inherit inappropriate methods, so `Json` could be converted to `Forward` which makes no sense.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #8279: Make realtime indexes pluggable
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8279:
URL: https://github.com/apache/pinot/pull/8279#discussion_r819795953
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentImpl.java
##########
@@ -242,6 +235,10 @@ public long getLatestIngestionTimestamp() {
// Initialize for each column
for (FieldSpec fieldSpec : _physicalFieldSpecs) {
String column = fieldSpec.getName();
+ boolean isRaw = isNoDictionaryColumn(noDictionaryColumns, invertedIndexColumns, fieldSpec, column);
Review comment:
Changing the name doesn't change its truth value?!
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #8279: Make realtime indexes pluggable
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #8279:
URL: https://github.com/apache/pinot/pull/8279#discussion_r818913738
##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/mutable/provider/MutableForwardIndexProvider.java
##########
@@ -0,0 +1,26 @@
+/**
+ * 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.pinot.segment.spi.index.mutable.provider;
+
+import org.apache.pinot.segment.spi.index.mutable.MutableForwardIndex;
+
+
+public interface MutableForwardIndexProvider {
+ MutableForwardIndex newIndex(MutableIndexContext.Forward context);
Review comment:
Consider renaming it to `newForwardIndex` to be consistent with the immutable provider interfaces? Same for other files
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] sajjad-moradi commented on a change in pull request #8279: Make realtime indexes pluggable
Posted by GitBox <gi...@apache.org>.
sajjad-moradi commented on a change in pull request #8279:
URL: https://github.com/apache/pinot/pull/8279#discussion_r819789995
##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/mutable/provider/MutableForwardIndexProvider.java
##########
@@ -0,0 +1,26 @@
+/**
+ * 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.pinot.segment.spi.index.mutable.provider;
+
+import org.apache.pinot.segment.spi.index.mutable.MutableForwardIndex;
+
+
+public interface MutableForwardIndexProvider {
+ MutableForwardIndex newIndex(MutableIndexContext.Forward context);
Review comment:
This is subjective, but IMO `newForwardIndex` reads better and less ambiguous. It's clear here in the interface definition because of the parameter type, but it will be a bit ambiguous in places where it's being used and the parameter name doesn't have word forward in it - like the one you have here `context`.
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentImpl.java
##########
@@ -242,6 +235,10 @@ public long getLatestIngestionTimestamp() {
// Initialize for each column
for (FieldSpec fieldSpec : _physicalFieldSpecs) {
String column = fieldSpec.getName();
+ boolean isRaw = isNoDictionaryColumn(noDictionaryColumns, invertedIndexColumns, fieldSpec, column);
Review comment:
If you rename `isRaw` to `isNoDictionary`, you don't need to negate it in both places it's being used and IMO it reads better.
##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/mutable/provider/MutableIndexContext.java
##########
@@ -0,0 +1,249 @@
+/**
+ * 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.pinot.segment.spi.index.mutable.provider;
+
+import java.util.Objects;
+import org.apache.pinot.segment.spi.memory.PinotDataBufferMemoryManager;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public interface MutableIndexContext {
+ PinotDataBufferMemoryManager getMemoryManager();
+
+ FieldSpec getFieldSpec();
+
+ String getSegmentName();
+
+ boolean hasDictionary();
+
+ int getCapacity();
+
+ boolean isOffHeap();
+
+ static Builder builder() {
+ return new Builder();
+ }
+
+ class Builder {
+ private FieldSpec _fieldSpec;
+ private String _segmentName;
+ private boolean _hasDictionary = true;
+ private boolean _offHeap = true;
+ private int _capacity;
+ private PinotDataBufferMemoryManager _memoryManager;
+
+ public Builder withMemoryManager(PinotDataBufferMemoryManager memoryManager) {
+ _memoryManager = memoryManager;
+ return this;
+ }
+
+ public Builder withFieldSpec(FieldSpec fieldSpec) {
+ _fieldSpec = fieldSpec;
+ return this;
+ }
+
+ public Builder withSegmentName(String segmentName) {
+ _segmentName = segmentName;
+ return this;
+ }
+
+ public Builder withDictionary(boolean hasDictionary) {
+ _hasDictionary = hasDictionary;
+ return this;
+ }
+
+ public Builder offHeap(boolean offHeap) {
+ _offHeap = offHeap;
+ return this;
+ }
+
+ public Builder withCapacity(int capacity) {
+ _capacity = capacity;
+ return this;
+ }
+
+ public Common build() {
+ return new Common(Objects.requireNonNull(_fieldSpec), _hasDictionary, Objects.requireNonNull(_segmentName),
+ Objects.requireNonNull(_memoryManager), _capacity, _offHeap);
+ }
+ }
+
+ final class Common implements MutableIndexContext {
+ private final int _capacity;
+ private final FieldSpec _fieldSpec;
+ private final boolean _hasDictionary;
+ private final boolean _offHeap;
+ private final String _segmentName;
+ private final PinotDataBufferMemoryManager _memoryManager;
+
+ public Common(FieldSpec fieldSpec, boolean hasDictionary, String segmentName,
+ PinotDataBufferMemoryManager memoryManager, int capacity, boolean offHeap) {
+ _fieldSpec = fieldSpec;
+ _hasDictionary = hasDictionary;
+ _segmentName = segmentName;
+ _memoryManager = memoryManager;
+ _capacity = capacity;
+ _offHeap = offHeap;
+ }
+
+ @Override
+ public PinotDataBufferMemoryManager getMemoryManager() {
+ return _memoryManager;
+ }
+
+ @Override
+ public String getSegmentName() {
+ return _segmentName;
+ }
+
+ @Override
+ public FieldSpec getFieldSpec() {
+ return _fieldSpec;
+ }
+
+ @Override
+ public boolean hasDictionary() {
+ return _hasDictionary;
+ }
+
+ @Override
+ public int getCapacity() {
+ return _capacity;
+ }
+
+ @Override
+ public boolean isOffHeap() {
+ return _offHeap;
+ }
+
+ public Dictionary forDictionary(int estimatedColSize, int estimatedCardinality) {
+ return new Dictionary(this, estimatedColSize, estimatedCardinality);
+ }
+
+ public Forward forForwardIndex(int avgNumMultiValues) {
+ return new Forward(this, avgNumMultiValues);
+ }
+
+ public Inverted forInvertedIndex() {
+ return new Inverted(this);
+ }
+
+ public Json forJsonIndex() {
+ return new Json(this);
+ }
+
+ public Text forTextIndex() {
+ return new Text(this);
+ }
+ }
+
+ class Wrapper implements MutableIndexContext {
Review comment:
Why do you need Wrapper class? Dictionary, Inverted, Json, etc can directly extend Common!
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #8279: Make realtime indexes pluggable
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8279:
URL: https://github.com/apache/pinot/pull/8279#discussion_r819820643
##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/mutable/provider/MutableIndexContext.java
##########
@@ -0,0 +1,249 @@
+/**
+ * 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.pinot.segment.spi.index.mutable.provider;
+
+import java.util.Objects;
+import org.apache.pinot.segment.spi.memory.PinotDataBufferMemoryManager;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public interface MutableIndexContext {
+ PinotDataBufferMemoryManager getMemoryManager();
+
+ FieldSpec getFieldSpec();
+
+ String getSegmentName();
+
+ boolean hasDictionary();
+
+ int getCapacity();
+
+ boolean isOffHeap();
+
+ static Builder builder() {
+ return new Builder();
+ }
+
+ class Builder {
+ private FieldSpec _fieldSpec;
+ private String _segmentName;
+ private boolean _hasDictionary = true;
+ private boolean _offHeap = true;
+ private int _capacity;
+ private PinotDataBufferMemoryManager _memoryManager;
+
+ public Builder withMemoryManager(PinotDataBufferMemoryManager memoryManager) {
+ _memoryManager = memoryManager;
+ return this;
+ }
+
+ public Builder withFieldSpec(FieldSpec fieldSpec) {
+ _fieldSpec = fieldSpec;
+ return this;
+ }
+
+ public Builder withSegmentName(String segmentName) {
+ _segmentName = segmentName;
+ return this;
+ }
+
+ public Builder withDictionary(boolean hasDictionary) {
+ _hasDictionary = hasDictionary;
+ return this;
+ }
+
+ public Builder offHeap(boolean offHeap) {
+ _offHeap = offHeap;
+ return this;
+ }
+
+ public Builder withCapacity(int capacity) {
+ _capacity = capacity;
+ return this;
+ }
+
+ public Common build() {
+ return new Common(Objects.requireNonNull(_fieldSpec), _hasDictionary, Objects.requireNonNull(_segmentName),
+ Objects.requireNonNull(_memoryManager), _capacity, _offHeap);
+ }
+ }
+
+ final class Common implements MutableIndexContext {
+ private final int _capacity;
+ private final FieldSpec _fieldSpec;
+ private final boolean _hasDictionary;
+ private final boolean _offHeap;
+ private final String _segmentName;
+ private final PinotDataBufferMemoryManager _memoryManager;
+
+ public Common(FieldSpec fieldSpec, boolean hasDictionary, String segmentName,
+ PinotDataBufferMemoryManager memoryManager, int capacity, boolean offHeap) {
+ _fieldSpec = fieldSpec;
+ _hasDictionary = hasDictionary;
+ _segmentName = segmentName;
+ _memoryManager = memoryManager;
+ _capacity = capacity;
+ _offHeap = offHeap;
+ }
+
+ @Override
+ public PinotDataBufferMemoryManager getMemoryManager() {
+ return _memoryManager;
+ }
+
+ @Override
+ public String getSegmentName() {
+ return _segmentName;
+ }
+
+ @Override
+ public FieldSpec getFieldSpec() {
+ return _fieldSpec;
+ }
+
+ @Override
+ public boolean hasDictionary() {
+ return _hasDictionary;
+ }
+
+ @Override
+ public int getCapacity() {
+ return _capacity;
+ }
+
+ @Override
+ public boolean isOffHeap() {
+ return _offHeap;
+ }
+
+ public Dictionary forDictionary(int estimatedColSize, int estimatedCardinality) {
+ return new Dictionary(this, estimatedColSize, estimatedCardinality);
+ }
+
+ public Forward forForwardIndex(int avgNumMultiValues) {
+ return new Forward(this, avgNumMultiValues);
+ }
+
+ public Inverted forInvertedIndex() {
+ return new Inverted(this);
+ }
+
+ public Json forJsonIndex() {
+ return new Json(this);
+ }
+
+ public Text forTextIndex() {
+ return new Text(this);
+ }
+ }
+
+ class Wrapper implements MutableIndexContext {
Review comment:
This is consistent with the pattern adopted for immutable indexes, and discussion like these tend to be six of one and half a dozen of the other. I remain unconvinced this needs to change.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #8279: Make realtime indexes pluggable
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #8279:
URL: https://github.com/apache/pinot/pull/8279#issuecomment-1057301185
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8279](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27214cb) into [master](https://codecov.io/gh/apache/pinot/commit/fd9c58a11ed16d27109baefcee138eea30132ad3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fd9c58a) will **decrease** coverage by `43.24%`.
> The diff coverage is `0.00%`.
> :exclamation: Current head 27214cb differs from pull request most recent head e7dfaff. Consider uploading reports for the commit e7dfaff to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8279 +/- ##
=============================================
- Coverage 70.84% 27.59% -43.25%
=============================================
Files 1631 1621 -10
Lines 85461 85204 -257
Branches 12877 12843 -34
=============================================
- Hits 60543 23513 -37030
- Misses 20735 59485 +38750
+ Partials 4183 2206 -1977
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `27.59% <0.00%> (+0.01%)` | :arrow_up: |
| unittests1 | `?` | |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...manager/realtime/LLRealtimeSegmentDataManager.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvTExSZWFsdGltZVNlZ21lbnREYXRhTWFuYWdlci5qYXZh) | `56.51% <ø> (-14.20%)` | :arrow_down: |
| [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `50.00% <ø> (ø)` | |
| [...ata/manager/realtime/RealtimeTableDataManager.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVUYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `66.26% <ø> (-2.01%)` | :arrow_down: |
| [...ava/org/apache/pinot/core/plan/FilterPlanNode.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0ZpbHRlclBsYW5Ob2RlLmphdmE=) | `54.20% <ø> (-33.65%)` | :arrow_down: |
| [...ocal/indexsegment/immutable/EmptyIndexSegment.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvaW1tdXRhYmxlL0VtcHR5SW5kZXhTZWdtZW50LmphdmE=) | `0.00% <ø> (-33.34%)` | :arrow_down: |
| [...l/indexsegment/immutable/ImmutableSegmentImpl.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvaW1tdXRhYmxlL0ltbXV0YWJsZVNlZ21lbnRJbXBsLmphdmE=) | `0.00% <ø> (-53.13%)` | :arrow_down: |
| [...exsegment/mutable/DefaultMutableIndexProvider.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvbXV0YWJsZS9EZWZhdWx0TXV0YWJsZUluZGV4UHJvdmlkZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ocal/indexsegment/mutable/IntermediateSegment.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvbXV0YWJsZS9JbnRlcm1lZGlhdGVTZWdtZW50LmphdmE=) | `0.00% <ø> (-69.63%)` | :arrow_down: |
| [...local/indexsegment/mutable/MutableSegmentImpl.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvbXV0YWJsZS9NdXRhYmxlU2VnbWVudEltcGwuamF2YQ==) | `0.00% <0.00%> (-59.22%)` | :arrow_down: |
| [...eaderwriter/RealtimeIndexOffHeapMemoryManager.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby9yZWFkZXJ3cml0ZXIvUmVhbHRpbWVJbmRleE9mZkhlYXBNZW1vcnlNYW5hZ2VyLmphdmE=) | `0.00% <ø> (-80.65%)` | :arrow_down: |
| ... and [1229 more](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [fd9c58a...e7dfaff](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #8279: Make realtime indexes pluggable
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8279:
URL: https://github.com/apache/pinot/pull/8279#discussion_r818921189
##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/mutable/provider/MutableForwardIndexProvider.java
##########
@@ -0,0 +1,26 @@
+/**
+ * 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.pinot.segment.spi.index.mutable.provider;
+
+import org.apache.pinot.segment.spi.index.mutable.MutableForwardIndex;
+
+
+public interface MutableForwardIndexProvider {
+ MutableForwardIndex newIndex(MutableIndexContext.Forward context);
Review comment:
I was thinking of changing the names of the other methods since the overloads can all be disambiguated by the parameter types, I felt this might be a neater convention. The other methods are not widely used yet so I don't think it's too late to change them.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on pull request #8279: Make realtime indexes pluggable
Posted by GitBox <gi...@apache.org>.
richardstartin commented on pull request #8279:
URL: https://github.com/apache/pinot/pull/8279#issuecomment-1059425866
> > @sajjad-moradi We rarely block a PR unless the code design is completely broken. I looked at the comments and I see that you have mentioned that most of them are subjective.
> > Can you please unblock us if what you are requesting is subjective. Thanks for understanding.
>
> Of course. Didn't know about this convention. The PR really looks good in general.
Thanks, I felt it was important to run the changes past you or @mcvsubbu as a courtesy, because you might need to debug something introduced here, but when it comes to naming (or composition vs inheritance) there tend to be as many opinions as there are people.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] sajjad-moradi commented on a change in pull request #8279: Make realtime indexes pluggable
Posted by GitBox <gi...@apache.org>.
sajjad-moradi commented on a change in pull request #8279:
URL: https://github.com/apache/pinot/pull/8279#discussion_r819809010
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentImpl.java
##########
@@ -242,6 +235,10 @@ public long getLatestIngestionTimestamp() {
// Initialize for each column
for (FieldSpec fieldSpec : _physicalFieldSpecs) {
String column = fieldSpec.getName();
+ boolean isRaw = isNoDictionaryColumn(noDictionaryColumns, invertedIndexColumns, fieldSpec, column);
Review comment:
You can go either way, but IMO they're essentially the same. The method name is `isNoDictionaryColumn()`, so it's more consistent with that too.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] kishoreg commented on pull request #8279: Make realtime indexes pluggable
Posted by GitBox <gi...@apache.org>.
kishoreg commented on pull request #8279:
URL: https://github.com/apache/pinot/pull/8279#issuecomment-1059417580
@sajjad-moradi We rarely block a PR unless the code design is completely broken. I looked at the comments and I see that you have mentioned that most of them are subjective.
Can you please unblock us if what you are requesting is subjective. Thanks for understanding.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #8279: Make realtime indexes pluggable
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8279:
URL: https://github.com/apache/pinot/pull/8279#issuecomment-1057301185
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8279](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4a77d8a) into [master](https://codecov.io/gh/apache/pinot/commit/e498a71bfc07e7f6ec163c61c4c14aab2e1a5ac9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e498a71) will **decrease** coverage by `3.80%`.
> The diff coverage is `86.76%`.
> :exclamation: Current head 4a77d8a differs from pull request most recent head f07e794. Consider uploading reports for the commit f07e794 to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8279 +/- ##
============================================
- Coverage 70.82% 67.01% -3.81%
+ Complexity 4239 4171 -68
============================================
Files 1631 1233 -398
Lines 85461 62354 -23107
Branches 12877 9703 -3174
============================================
- Hits 60527 41787 -18740
+ Misses 20749 17601 -3148
+ Partials 4185 2966 -1219
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `67.01% <86.76%> (+0.09%)` | :arrow_up: |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...dindex/RealtimeLuceneIndexReaderRefreshThread.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2ludmVydGVkaW5kZXgvUmVhbHRpbWVMdWNlbmVJbmRleFJlYWRlclJlZnJlc2hUaHJlYWQuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...invertedindex/RealtimeLuceneIndexRefreshState.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2ludmVydGVkaW5kZXgvUmVhbHRpbWVMdWNlbmVJbmRleFJlZnJlc2hTdGF0ZS5qYXZh) | `0.00% <ø> (ø)` | |
| [...me/impl/invertedindex/RealtimeLuceneTextIndex.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2ludmVydGVkaW5kZXgvUmVhbHRpbWVMdWNlbmVUZXh0SW5kZXguamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ment/creator/impl/text/LuceneTextIndexCreator.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC90ZXh0L0x1Y2VuZVRleHRJbmRleENyZWF0b3IuamF2YQ==) | `76.59% <ø> (ø)` | |
| [...che/pinot/segment/spi/index/IndexingOverrides.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L0luZGV4aW5nT3ZlcnJpZGVzLmphdmE=) | `91.54% <76.47%> (-4.82%)` | :arrow_down: |
| [...exsegment/mutable/DefaultMutableIndexProvider.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvbXV0YWJsZS9EZWZhdWx0TXV0YWJsZUluZGV4UHJvdmlkZXIuamF2YQ==) | `82.85% <82.85%> (ø)` | |
| [...local/indexsegment/mutable/MutableSegmentImpl.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvbXV0YWJsZS9NdXRhYmxlU2VnbWVudEltcGwuamF2YQ==) | `58.73% <93.75%> (-0.49%)` | :arrow_down: |
| [...pi/index/mutable/provider/MutableIndexContext.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L211dGFibGUvcHJvdmlkZXIvTXV0YWJsZUluZGV4Q29udGV4dC5qYXZh) | `95.16% <95.16%> (ø)` | |
| [...time/impl/invertedindex/RealtimeInvertedIndex.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2ludmVydGVkaW5kZXgvUmVhbHRpbWVJbnZlcnRlZEluZGV4LmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...local/realtime/impl/json/MutableJsonIndexImpl.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2pzb24vTXV0YWJsZUpzb25JbmRleEltcGwuamF2YQ==) | `86.40% <100.00%> (ø)` | |
| ... and [631 more](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e498a71...f07e794](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #8279: Make realtime indexes pluggable
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8279:
URL: https://github.com/apache/pinot/pull/8279#issuecomment-1057301185
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8279](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27214cb) into [master](https://codecov.io/gh/apache/pinot/commit/fd9c58a11ed16d27109baefcee138eea30132ad3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fd9c58a) will **decrease** coverage by `40.12%`.
> The diff coverage is `0.00%`.
> :exclamation: Current head 27214cb differs from pull request most recent head e7dfaff. Consider uploading reports for the commit e7dfaff to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8279 +/- ##
=============================================
- Coverage 70.84% 30.71% -40.13%
=============================================
Files 1631 1621 -10
Lines 85461 85204 -257
Branches 12877 12843 -34
=============================================
- Hits 60543 26172 -34371
- Misses 20735 56682 +35947
+ Partials 4183 2350 -1833
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `28.94% <0.00%> (+0.01%)` | :arrow_up: |
| integration2 | `27.59% <0.00%> (+0.01%)` | :arrow_up: |
| unittests1 | `?` | |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...manager/realtime/LLRealtimeSegmentDataManager.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvTExSZWFsdGltZVNlZ21lbnREYXRhTWFuYWdlci5qYXZh) | `58.46% <ø> (-12.24%)` | :arrow_down: |
| [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `50.00% <ø> (ø)` | |
| [...ata/manager/realtime/RealtimeTableDataManager.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVUYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `67.46% <ø> (-0.81%)` | :arrow_down: |
| [...ava/org/apache/pinot/core/plan/FilterPlanNode.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0ZpbHRlclBsYW5Ob2RlLmphdmE=) | `55.14% <ø> (-32.72%)` | :arrow_down: |
| [...ocal/indexsegment/immutable/EmptyIndexSegment.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvaW1tdXRhYmxlL0VtcHR5SW5kZXhTZWdtZW50LmphdmE=) | `0.00% <ø> (-33.34%)` | :arrow_down: |
| [...l/indexsegment/immutable/ImmutableSegmentImpl.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvaW1tdXRhYmxlL0ltbXV0YWJsZVNlZ21lbnRJbXBsLmphdmE=) | `0.00% <ø> (-53.13%)` | :arrow_down: |
| [...exsegment/mutable/DefaultMutableIndexProvider.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvbXV0YWJsZS9EZWZhdWx0TXV0YWJsZUluZGV4UHJvdmlkZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ocal/indexsegment/mutable/IntermediateSegment.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvbXV0YWJsZS9JbnRlcm1lZGlhdGVTZWdtZW50LmphdmE=) | `0.00% <ø> (-69.63%)` | :arrow_down: |
| [...local/indexsegment/mutable/MutableSegmentImpl.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvbXV0YWJsZS9NdXRhYmxlU2VnbWVudEltcGwuamF2YQ==) | `0.00% <0.00%> (-59.22%)` | :arrow_down: |
| [...eaderwriter/RealtimeIndexOffHeapMemoryManager.java](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby9yZWFkZXJ3cml0ZXIvUmVhbHRpbWVJbmRleE9mZkhlYXBNZW1vcnlNYW5hZ2VyLmphdmE=) | `0.00% <ø> (-80.65%)` | :arrow_down: |
| ... and [1163 more](https://codecov.io/gh/apache/pinot/pull/8279/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [fd9c58a...e7dfaff](https://codecov.io/gh/apache/pinot/pull/8279?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] sajjad-moradi commented on pull request #8279: Make realtime indexes pluggable
Posted by GitBox <gi...@apache.org>.
sajjad-moradi commented on pull request #8279:
URL: https://github.com/apache/pinot/pull/8279#issuecomment-1059424095
> @sajjad-moradi We rarely block a PR unless the code design is completely broken. I looked at the comments and I see that you have mentioned that most of them are subjective.
>
> Can you please unblock us if what you are requesting is subjective. Thanks for understanding.
Of course. Didn't know about this convention. The PR really looks good in general.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org