Harnoor7 commented on code in PR #14479:
URL: https://github.com/apache/pinot/pull/14479#discussion_r1858294143
##########
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/IndexingConfig.java:
##########
@@ -410,6 +411,14 @@ public void setSegmentNameGeneratorType(String
segmentNameGeneratorType) {
_segmentNameGeneratorType = segmentNameGeneratorType;
}
+ public boolean isIndexCapacityThresholdCheckEnabled() {
+ return _indexCapacityThresholdCheckEnabled;
+ }
+
+ public void setIndexCapacityThresholdCheckEnabled(boolean
indexCapacityThresholdCheckEnabled) {
+ _indexCapacityThresholdCheckEnabled = indexCapacityThresholdCheckEnabled;
+ }
+
Review Comment:
> We may also want to set this for all tables
Did we encounter such scenario where there was need for this? In my
knowledge, I am only aware of this issue happening only 1 time for 1 table only
(rare).
Making this change as default can be dangerous (can create high num of
segments in cluster) since we are taking a conservative approach over here.
There are indexes like FixedBitMVEntryDictForwardIndex,
VarByteChunkForwardIndex, etc which are capable of holding more than 450
million num of entries and more than 2GB of data.
For future PR, I think we should just work on making FixedBitMVForwardIndex
unbounded and then we can enable this flag for default.
##########
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/IndexingConfig.java:
##########
@@ -410,6 +411,14 @@ public void setSegmentNameGeneratorType(String
segmentNameGeneratorType) {
_segmentNameGeneratorType = segmentNameGeneratorType;
}
+ public boolean isIndexCapacityThresholdCheckEnabled() {
+ return _indexCapacityThresholdCheckEnabled;
+ }
+
+ public void setIndexCapacityThresholdCheckEnabled(boolean
indexCapacityThresholdCheckEnabled) {
+ _indexCapacityThresholdCheckEnabled = indexCapacityThresholdCheckEnabled;
+ }
+
Review Comment:
> We may also want to set this for all tables
Did we encounter such scenario where there was need for this? In my
knowledge, I am only aware of this issue happening only 1 time for 1 table only
(rare).
Making this change as default can be dangerous (can create high num of
segments in cluster) since we are taking a conservative approach over here.
There are indexes like FixedBitMVEntryDictForwardIndex,
VarByteChunkForwardIndex, etc which are capable of holding more than 450
million num of entries and more than 2GB of data.
For future PR, I think we should just work on making FixedBitMVForwardIndex
unbounded and then we can enable this flag for default.
--
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-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
