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 2021/12/23 10:54:20 UTC
[GitHub] [pinot] mneedham opened a new pull request #7956: Pull out segment flush threshold computation to make it easier to test
mneedham opened a new pull request #7956:
URL: https://github.com/apache/pinot/pull/7956
## Description
I wanted to understand how the segment-based flush threshold updater works, so I pulled out the computation bit of code so that I could write some tests to see how different inputs affect the threshold.
I haven't changed the way it works - only pulled out code from `SegmentSizeBasedFlushThresholdUpdater` into `SegmentFlushThresholdComputer`.
## Upgrade Notes
Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR fix a zero-downtime upgrade introduced earlier?
* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR otherwise need attention when creating release notes? Things to consider:
- New configuration options
- Deprecation of configurations
- Signature changes to public methods/interfaces
- New plugins added or old plugins removed
* [ ] Yes (Please label this PR as **<code>release-notes</code>** and complete the section on Release Notes)
## Release Notes
<!-- If you have tagged this as either backward-incompat or release-notes,
you MUST add text here that you would like to see appear in release notes of the
next release. -->
<!-- If you have a series of commits adding or enabling a feature, then
add this section only in final commit that marks the feature completed.
Refer to earlier release notes to see examples of text.
-->
## Documentation
<!-- If you have introduced a new feature or configuration, please add it to the documentation as well.
See https://docs.pinot.apache.org/developers/developers-and-contributors/update-document
-->
--
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] mcvsubbu commented on a change in pull request #7956: Pull out segment flush threshold computation to make it easier to test
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #7956:
URL: https://github.com/apache/pinot/pull/7956#discussion_r774696717
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/segment/SegmentFlushThresholdComputer.java
##########
@@ -0,0 +1,182 @@
+/**
+ * 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.controller.helix.core.realtime.segment;
+
+import java.time.Clock;
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.metadata.segment.SegmentZKMetadata;
+import org.apache.pinot.common.utils.LLCSegmentName;
+import org.apache.pinot.spi.stream.PartitionGroupMetadata;
+import org.apache.pinot.spi.stream.PartitionLevelStreamConfig;
+import org.apache.pinot.spi.utils.TimeUtils;
+
+public class SegmentFlushThresholdComputer {
+ public static final int MINIMUM_NUM_ROWS_THRESHOLD = 10_000;
Review comment:
Doesnt need to be public, right?
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/segment/SegmentFlushThresholdComputer.java
##########
@@ -0,0 +1,182 @@
+/**
+ * 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.controller.helix.core.realtime.segment;
+
+import java.time.Clock;
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.metadata.segment.SegmentZKMetadata;
+import org.apache.pinot.common.utils.LLCSegmentName;
+import org.apache.pinot.spi.stream.PartitionGroupMetadata;
+import org.apache.pinot.spi.stream.PartitionLevelStreamConfig;
+import org.apache.pinot.spi.utils.TimeUtils;
+
+public class SegmentFlushThresholdComputer {
+ public static final int MINIMUM_NUM_ROWS_THRESHOLD = 10_000;
+ static final double CURRENT_SEGMENT_RATIO_WEIGHT = 0.1;
+ static final double PREVIOUS_SEGMENT_RATIO_WEIGHT = 0.9;
+ static final double ROWS_MULTIPLIER_WHEN_TIME_THRESHOLD_HIT = 1.1;
+
+ // num rows to segment size ratio of last committed segment for this table
+ private double _latestSegmentRowsToSizeRatio;
+ private final Clock _clock;
+
+ public SegmentFlushThresholdComputer() {
+ this(Clock.systemUTC(), 0);
+ }
+
+ public SegmentFlushThresholdComputer(Clock clock, double latestSegmentRowsToSizeRatio) {
Review comment:
please annotate this constructor and visble for testing
--
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 #7956: Pull out segment flush threshold computation to make it easier to test
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7956:
URL: https://github.com/apache/pinot/pull/7956#issuecomment-1000261734
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7956?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 [#7956](https://codecov.io/gh/apache/pinot/pull/7956?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f4e4f81) into [master](https://codecov.io/gh/apache/pinot/commit/b6eeaf3cf3fbc4592916c5d9db12ae9f9798db6a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b6eeaf3) will **decrease** coverage by `6.37%`.
> The diff coverage is `96.15%`.
[](https://codecov.io/gh/apache/pinot/pull/7956?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 #7956 +/- ##
============================================
- Coverage 71.20% 64.82% -6.38%
+ Complexity 4174 4172 -2
============================================
Files 1593 1549 -44
Lines 82477 80614 -1863
Branches 12305 12103 -202
============================================
- Hits 58729 52260 -6469
- Misses 19788 24639 +4851
+ Partials 3960 3715 -245
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `68.15% <ø> (-0.04%)` | :arrow_down: |
| unittests2 | `14.33% <96.15%> (+0.02%)` | :arrow_up: |
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/7956?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ealtime/segment/SegmentFlushThresholdComputer.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL3NlZ21lbnQvU2VnbWVudEZsdXNoVGhyZXNob2xkQ29tcHV0ZXIuamF2YQ==) | `95.71% <95.71%> (ø)` | |
| [...segment/SegmentSizeBasedFlushThresholdUpdater.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL3NlZ21lbnQvU2VnbWVudFNpemVCYXNlZEZsdXNoVGhyZXNob2xkVXBkYXRlci5qYXZh) | `100.00% <100.00%> (+28.16%)` | :arrow_up: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvVGltZXJTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...not/common/exception/HttpErrorStatusException.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL0h0dHBFcnJvclN0YXR1c0V4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [368 more](https://codecov.io/gh/apache/pinot/pull/7956/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/7956?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/7956?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 [b6eeaf3...f4e4f81](https://codecov.io/gh/apache/pinot/pull/7956?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 #7956: Pull out segment flush threshold computation to make it easier to test
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7956:
URL: https://github.com/apache/pinot/pull/7956#issuecomment-1000261734
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7956?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 [#7956](https://codecov.io/gh/apache/pinot/pull/7956?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3dc4302) into [master](https://codecov.io/gh/apache/pinot/commit/b6eeaf3cf3fbc4592916c5d9db12ae9f9798db6a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b6eeaf3) will **decrease** coverage by `56.87%`.
> The diff coverage is `33.03%`.
> :exclamation: Current head 3dc4302 differs from pull request most recent head d9db3a6. Consider uploading reports for the commit d9db3a6 to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7956?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 #7956 +/- ##
=============================================
- Coverage 71.20% 14.33% -56.88%
+ Complexity 4174 80 -4094
=============================================
Files 1593 1551 -42
Lines 82477 80717 -1760
Branches 12305 12120 -185
=============================================
- Hits 58729 11571 -47158
- Misses 19788 68290 +48502
+ Partials 3960 856 -3104
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.33% <33.03%> (+0.02%)` | :arrow_up: |
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/7956?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...org/apache/pinot/sql/parsers/CalciteSqlParser.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvcGFyc2Vycy9DYWxjaXRlU3FsUGFyc2VyLmphdmE=) | `0.00% <0.00%> (-87.29%)` | :arrow_down: |
| [...ces/PinotSegmentUploadDownloadRestletResource.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90U2VnbWVudFVwbG9hZERvd25sb2FkUmVzdGxldFJlc291cmNlLmphdmE=) | `39.56% <0.00%> (-18.70%)` | :arrow_down: |
| [.../core/realtime/PinotLLCRealtimeSegmentManager.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90TExDUmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh) | `67.68% <0.00%> (-11.29%)` | :arrow_down: |
| [...ot/core/query/scheduler/QuerySchedulerFactory.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zY2hlZHVsZXIvUXVlcnlTY2hlZHVsZXJGYWN0b3J5LmphdmE=) | `0.00% <0.00%> (-28.00%)` | :arrow_down: |
| [...ore/query/scheduler/fcfs/BoundedFCFSScheduler.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zY2hlZHVsZXIvZmNmcy9Cb3VuZGVkRkNGU1NjaGVkdWxlci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [.../core/query/scheduler/fcfs/FCFSQueryScheduler.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zY2hlZHVsZXIvZmNmcy9GQ0ZTUXVlcnlTY2hlZHVsZXIuamF2YQ==) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [...pache/pinot/core/util/SegmentRefreshSemaphore.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1NlZ21lbnRSZWZyZXNoU2VtYXBob3JlLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [.../io/writer/impl/BaseChunkSVForwardIndexWriter.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby93cml0ZXIvaW1wbC9CYXNlQ2h1bmtTVkZvcndhcmRJbmRleFdyaXRlci5qYXZh) | `0.00% <0.00%> (-85.97%)` | :arrow_down: |
| [...riter/impl/FixedByteChunkSVForwardIndexWriter.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby93cml0ZXIvaW1wbC9GaXhlZEJ5dGVDaHVua1NWRm9yd2FyZEluZGV4V3JpdGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../writer/impl/VarByteChunkSVForwardIndexWriter.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby93cml0ZXIvaW1wbC9WYXJCeXRlQ2h1bmtTVkZvcndhcmRJbmRleFdyaXRlci5qYXZh) | `0.00% <ø> (-100.00%)` | :arrow_down: |
| ... and [1277 more](https://codecov.io/gh/apache/pinot/pull/7956/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/7956?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/7956?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 [b6eeaf3...d9db3a6](https://codecov.io/gh/apache/pinot/pull/7956?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 #7956: Pull out segment flush threshold computation to make it easier to test
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7956:
URL: https://github.com/apache/pinot/pull/7956#issuecomment-1000261734
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7956?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 [#7956](https://codecov.io/gh/apache/pinot/pull/7956?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f4e4f81) into [master](https://codecov.io/gh/apache/pinot/commit/b6eeaf3cf3fbc4592916c5d9db12ae9f9798db6a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b6eeaf3) will **decrease** coverage by `0.89%`.
> The diff coverage is `96.15%`.
[](https://codecov.io/gh/apache/pinot/pull/7956?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 #7956 +/- ##
============================================
- Coverage 71.20% 70.31% -0.90%
+ Complexity 4174 4172 -2
============================================
Files 1593 1594 +1
Lines 82477 82485 +8
Branches 12305 12305
============================================
- Hits 58729 57996 -733
- Misses 19788 20523 +735
- Partials 3960 3966 +6
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `28.99% <0.00%> (-0.08%)` | :arrow_down: |
| integration2 | `?` | |
| unittests1 | `68.15% <ø> (-0.04%)` | :arrow_down: |
| unittests2 | `14.33% <96.15%> (+0.02%)` | :arrow_up: |
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/7956?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ealtime/segment/SegmentFlushThresholdComputer.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL3NlZ21lbnQvU2VnbWVudEZsdXNoVGhyZXNob2xkQ29tcHV0ZXIuamF2YQ==) | `95.71% <95.71%> (ø)` | |
| [...segment/SegmentSizeBasedFlushThresholdUpdater.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL3NlZ21lbnQvU2VnbWVudFNpemVCYXNlZEZsdXNoVGhyZXNob2xkVXBkYXRlci5qYXZh) | `100.00% <100.00%> (+28.16%)` | :arrow_up: |
| [...t/core/plan/StreamingInstanceResponsePlanNode.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL1N0cmVhbWluZ0luc3RhbmNlUmVzcG9uc2VQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ore/operator/streaming/StreamingResponseUtils.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9zdHJlYW1pbmcvU3RyZWFtaW5nUmVzcG9uc2VVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ager/realtime/PeerSchemeSplitSegmentCommitter.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUGVlclNjaGVtZVNwbGl0U2VnbWVudENvbW1pdHRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvZ3JwYy9HcnBjUXVlcnlDbGllbnQuamF2YQ==) | `0.00% <0.00%> (-94.74%)` | :arrow_down: |
| [...he/pinot/core/plan/StreamingSelectionPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL1N0cmVhbWluZ1NlbGVjdGlvblBsYW5Ob2RlLmphdmE=) | `0.00% <0.00%> (-88.89%)` | :arrow_down: |
| [...ator/streaming/StreamingSelectionOnlyOperator.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9zdHJlYW1pbmcvU3RyZWFtaW5nU2VsZWN0aW9uT25seU9wZXJhdG9yLmphdmE=) | `0.00% <0.00%> (-87.81%)` | :arrow_down: |
| [...data/manager/realtime/DefaultSegmentCommitter.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvRGVmYXVsdFNlZ21lbnRDb21taXR0ZXIuamF2YQ==) | `0.00% <0.00%> (-80.00%)` | :arrow_down: |
| [...ller/api/access/BasicAuthAccessControlFactory.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvYWNjZXNzL0Jhc2ljQXV0aEFjY2Vzc0NvbnRyb2xGYWN0b3J5LmphdmE=) | `0.00% <0.00%> (-80.00%)` | :arrow_down: |
| ... and [92 more](https://codecov.io/gh/apache/pinot/pull/7956/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/7956?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/7956?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 [b6eeaf3...f4e4f81](https://codecov.io/gh/apache/pinot/pull/7956?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 #7956: Pull out segment flush threshold computation to make it easier to test
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #7956:
URL: https://github.com/apache/pinot/pull/7956#discussion_r774501653
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/segment/SegmentFlushThresholdComputer.java
##########
@@ -0,0 +1,184 @@
+/**
+ * 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.controller.helix.core.realtime.segment;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.metadata.segment.SegmentZKMetadata;
+import org.apache.pinot.common.utils.LLCSegmentName;
+import org.apache.pinot.spi.stream.PartitionGroupMetadata;
+import org.apache.pinot.spi.stream.PartitionLevelStreamConfig;
+import org.apache.pinot.spi.utils.TimeUtils;
+
+import static org.apache.pinot.controller.helix.core.realtime.segment.SegmentSizeBasedFlushThresholdUpdater.LOGGER;
+
+
+public class SegmentFlushThresholdComputer {
+ public static final int MINIMUM_NUM_ROWS_THRESHOLD = 10_000;
+ static final double CURRENT_SEGMENT_RATIO_WEIGHT = 0.1;
+ static final double PREVIOUS_SEGMENT_RATIO_WEIGHT = 0.9;
+ static final double ROWS_MULTIPLIER_WHEN_TIME_THRESHOLD_HIT = 1.1;
+
+ // num rows to segment size ratio of last committed segment for this table
+ private double _latestSegmentRowsToSizeRatio;
+ private final Clock _clock;
+
+ public SegmentFlushThresholdComputer() {
+ this(new SystemClock(), 0);
+ }
+
+ public SegmentFlushThresholdComputer(Clock clock, double latestSegmentRowsToSizeRatio) {
+ this._clock = clock;
+ this._latestSegmentRowsToSizeRatio = latestSegmentRowsToSizeRatio;
+ }
+
+ public SegmentFlushThresholdComputer(Clock clock) {
+ this(clock, 0);
+ }
+
+ double getLatestSegmentRowsToSizeRatio() {
+ return _latestSegmentRowsToSizeRatio;
+ }
+
+ public int computeThreshold(PartitionLevelStreamConfig streamConfig,
+ CommittingSegmentDescriptor committingSegmentDescriptor,
+ @Nullable SegmentZKMetadata committingSegmentZKMetadata,
+ List<PartitionGroupMetadata> partitionGroupMetadataList, String newSegmentName) {
+ final long desiredSegmentSizeBytes = streamConfig.getFlushThresholdSegmentSizeBytes();
+ final long optimalSegmentSizeBytesMin = desiredSegmentSizeBytes / 2;
+ final double optimalSegmentSizeBytesMax = desiredSegmentSizeBytes * 1.5;
+
+ if (committingSegmentZKMetadata == null) { // first segment of the partition, hence committing segment is null
+ if (_latestSegmentRowsToSizeRatio > 0) { // new partition group added case
+ long targetSegmentNumRows = (long) (desiredSegmentSizeBytes * _latestSegmentRowsToSizeRatio);
+ targetSegmentNumRows = capNumRowsIfOverflow(targetSegmentNumRows);
+ LOGGER.info(
+ "Committing segment zk metadata is not available, using prev ratio {}, setting rows threshold for {} as {}",
+ _latestSegmentRowsToSizeRatio, newSegmentName, targetSegmentNumRows);
+ return (int) targetSegmentNumRows;
+ } else {
+ final int autotuneInitialRows = streamConfig.getFlushAutotuneInitialRows();
+ LOGGER.info(
+ "Committing segment zk metadata is not available, setting threshold for {} as {}", newSegmentName,
+ autotuneInitialRows);
+ return autotuneInitialRows;
+ }
+ }
+
+ final long committingSegmentSizeBytes = committingSegmentDescriptor.getSegmentSizeBytes();
+ if (committingSegmentSizeBytes <= 0) { // repair segment case
+ final int targetNumRows = committingSegmentZKMetadata.getSizeThresholdToFlushSegment();
+ LOGGER.info(
+ "Committing segment size is not available, setting thresholds from previous segment for {} as {}",
+ newSegmentName, targetNumRows);
+ return targetNumRows;
+ }
+
+ final long timeConsumed = _clock.currentTimeMillis() - committingSegmentZKMetadata.getCreationTime();
+ final long numRowsConsumed = committingSegmentZKMetadata.getTotalDocs();
+ final int numRowsThreshold = committingSegmentZKMetadata.getSizeThresholdToFlushSegment();
+ LOGGER.info(
+ "{}: Data from committing segment: Time {} numRows {} threshold {} segmentSize(bytes) {}",
+ newSegmentName, TimeUtils.convertMillisToPeriod(timeConsumed), numRowsConsumed, numRowsThreshold,
+ committingSegmentSizeBytes);
+
+ double currentRatio = (double) numRowsConsumed / committingSegmentSizeBytes;
+ // Compute segment size to rows ratio only from the lowest available partition id.
+ // If we consider all partitions then it is likely that we will assign a much higher weight to the most
+ // recent trend in the table (since it is usually true that all partitions of the same table follow more or
+ // less same characteristics at any one point in time).
+ // However, when we start a new table or change controller mastership, we can have any partition completing first.
+ // It is best to learn the ratio as quickly as we can, so we allow any partition to supply the value.
+ int smallestAvailablePartitionGroupId =
+ partitionGroupMetadataList.stream().mapToInt(PartitionGroupMetadata::getPartitionGroupId).min()
+ .orElseGet(() -> 0);
Review comment:
I know this is just code moved from one place to another but `.orElse(0)` would be better
--
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] npawar commented on a change in pull request #7956: Pull out segment flush threshold computation to make it easier to test
Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #7956:
URL: https://github.com/apache/pinot/pull/7956#discussion_r782433859
##########
File path: pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/realtime/segment/SegmentFlushThresholdComputerTest.java
##########
@@ -0,0 +1,314 @@
+/**
+ * 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.controller.helix.core.realtime.segment;
+
+import java.time.Clock;
+import java.time.ZoneId;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import org.apache.pinot.common.metadata.segment.SegmentZKMetadata;
+import org.apache.pinot.spi.stream.PartitionGroupMetadata;
+import org.apache.pinot.spi.stream.PartitionLevelStreamConfig;
+import org.testng.annotations.Test;
+
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.testng.Assert.assertEquals;
+
+
+public class SegmentFlushThresholdComputerTest {
+ @Test
+ public void testUseAutoTuneInitialRowsIfFirstSegmentInPartition() {
+ int autoTuneInitialRows = 1_000;
+ SegmentFlushThresholdComputer computer = new SegmentFlushThresholdComputer();
+
+ PartitionLevelStreamConfig streamConfig = mock(PartitionLevelStreamConfig.class);
+ when(streamConfig.getFlushAutotuneInitialRows()).thenReturn(autoTuneInitialRows);
+
+ CommittingSegmentDescriptor committingSegmentDescriptor = mock(CommittingSegmentDescriptor.class);
+ SegmentZKMetadata committingSegmentZKMetadata = null;
+
+ List<PartitionGroupMetadata> partitionGroupMetadataList = new ArrayList<>();
+ int threshold = computer.computeThreshold(
+ streamConfig, committingSegmentDescriptor, committingSegmentZKMetadata,
+ partitionGroupMetadataList, "newSegmentName");
+
+ assertEquals(threshold, autoTuneInitialRows);
+ }
+
+ @Test
+ public void testUseLastSegmentSizeTimesRatioIfFirstSegmentInPartitionAndNewPartitionGroup() {
+ double segmentRowsToSizeRatio = 1.5;
+ long segmentSizeBytes = 20000L;
+ SegmentFlushThresholdComputer computer = new SegmentFlushThresholdComputer(
+ Clock.systemUTC(), segmentRowsToSizeRatio);
+
+ PartitionLevelStreamConfig streamConfig = mock(PartitionLevelStreamConfig.class);
+ when(streamConfig.getFlushThresholdSegmentSizeBytes()).thenReturn(segmentSizeBytes);
+
+ CommittingSegmentDescriptor committingSegmentDescriptor = mock(CommittingSegmentDescriptor.class);
+ SegmentZKMetadata committingSegmentZKMetadata = null;
+
+ List<PartitionGroupMetadata> partitionGroupMetadataList = new ArrayList<>();
+ int threshold = computer.computeThreshold(
+ streamConfig, committingSegmentDescriptor, committingSegmentZKMetadata,
+ partitionGroupMetadataList, "newSegmentName");
+
+ // segmentSize * 1.5
+ // 20000 * 1.5
+ assertEquals(threshold, 30000);
+ }
+
+ @Test
+ public void testUseLastSegmentSizeTimesRatioIfFirstSegmentInPartitionAndNewPartitionGroupMinimumSize10000Rows() {
+ double segmentRowsToSizeRatio = 1.5;
+ long segmentSizeBytes = 2000L;
+ SegmentFlushThresholdComputer computer = new SegmentFlushThresholdComputer(
+ Clock.systemUTC(), segmentRowsToSizeRatio);
+
+ PartitionLevelStreamConfig streamConfig = mock(PartitionLevelStreamConfig.class);
+ when(streamConfig.getFlushThresholdSegmentSizeBytes()).thenReturn(segmentSizeBytes);
+
+ CommittingSegmentDescriptor committingSegmentDescriptor = mock(CommittingSegmentDescriptor.class);
+ SegmentZKMetadata committingSegmentZKMetadata = null;
+
+ List<PartitionGroupMetadata> partitionGroupMetadataList = new ArrayList<>();
+ int threshold = computer.computeThreshold(
+ streamConfig, committingSegmentDescriptor, committingSegmentZKMetadata,
+ partitionGroupMetadataList, "newSegmentName");
+
+ assertEquals(threshold, 10000);
+ }
+
+ @Test
+ public void testUseLastSegmentsThresholdIfSegmentSizeMissing() {
+ long segmentSizeBytes = 0L;
+ int segmentSizeThreshold = 5_000;
+ SegmentFlushThresholdComputer computer = new SegmentFlushThresholdComputer();
+
+ PartitionLevelStreamConfig streamConfig = mock(PartitionLevelStreamConfig.class);
+ when(streamConfig.getFlushThresholdTimeMillis()).thenReturn(123L);
+
+ CommittingSegmentDescriptor committingSegmentDescriptor = mock(CommittingSegmentDescriptor.class);
+ when(committingSegmentDescriptor.getSegmentSizeBytes()).thenReturn(segmentSizeBytes);
+
+ SegmentZKMetadata committingSegmentZKMetadata = mock(SegmentZKMetadata.class);
+ when(committingSegmentZKMetadata.getSizeThresholdToFlushSegment()).thenReturn(segmentSizeThreshold);
+
+ List<PartitionGroupMetadata> partitionGroupMetadataList = new ArrayList<>();
+ int threshold = computer.computeThreshold(
+ streamConfig, committingSegmentDescriptor, committingSegmentZKMetadata,
+ partitionGroupMetadataList, "newSegmentName");
+
+ assertEquals(threshold, segmentSizeThreshold);
+ }
+
+ @Test
+ public void testApplyMultiplierToTotalDocsWhenTimeThresholdNotReached() {
+ long currentTime = 1640216032391L;
+ Clock clock = Clock.fixed(java.time.Instant.ofEpochMilli(currentTime), ZoneId.of("UTC"));
+
+ SegmentFlushThresholdComputer computer = new SegmentFlushThresholdComputer(clock);
+
+ PartitionLevelStreamConfig streamConfig = mock(PartitionLevelStreamConfig.class);
+ when(streamConfig.getFlushThresholdSegmentSizeBytes()).thenReturn(300_0000L);
+ when(streamConfig.getFlushThresholdTimeMillis()).thenReturn(
+ MILLISECONDS.convert(6, TimeUnit.HOURS));
+
+ CommittingSegmentDescriptor committingSegmentDescriptor = mock(CommittingSegmentDescriptor.class);
+ when(committingSegmentDescriptor.getSegmentSizeBytes()).thenReturn(200_0000L);
+
+ SegmentZKMetadata committingSegmentZKMetadata = mock(SegmentZKMetadata.class);
+ when(committingSegmentZKMetadata.getTotalDocs()).thenReturn(10_000L);
+ when(committingSegmentZKMetadata.getSizeThresholdToFlushSegment()).thenReturn(20_000);
+ when(committingSegmentZKMetadata.getCreationTime()).thenReturn(
+ currentTime - MILLISECONDS.convert(1, TimeUnit.HOURS));
+
+ List<PartitionGroupMetadata> partitionGroupMetadataList = new ArrayList<>();
+ int threshold = computer.computeThreshold(
+ streamConfig, committingSegmentDescriptor, committingSegmentZKMetadata,
+ partitionGroupMetadataList, "events3__0__0__20211222T1646Z");
+
+ // totalDocs * 1.1
+ // 10000 * 1.1
+ assertEquals(threshold, 11_000);
+ }
+
+ @Test
+ public void testApplyMultiplierToAdjustedTotalDocsWhenTimeThresholdIsReached() {
+ long currentTime = 1640216032391L;
+ Clock clock = Clock.fixed(java.time.Instant.ofEpochMilli(currentTime), ZoneId.of("UTC"));
+
+ SegmentFlushThresholdComputer computer = new SegmentFlushThresholdComputer(clock);
+
+ PartitionLevelStreamConfig streamConfig = mock(PartitionLevelStreamConfig.class);
+ when(streamConfig.getFlushThresholdSegmentSizeBytes()).thenReturn(300_0000L);
+ when(streamConfig.getFlushThresholdTimeMillis()).thenReturn(
+ MILLISECONDS.convert(1, TimeUnit.HOURS));
+
+ CommittingSegmentDescriptor committingSegmentDescriptor = mock(CommittingSegmentDescriptor.class);
+ when(committingSegmentDescriptor.getSegmentSizeBytes()).thenReturn(200_0000L);
+
+ SegmentZKMetadata committingSegmentZKMetadata = mock(SegmentZKMetadata.class);
+ when(committingSegmentZKMetadata.getTotalDocs()).thenReturn(30_000L);
+ when(committingSegmentZKMetadata.getSizeThresholdToFlushSegment()).thenReturn(60_000);
+ when(committingSegmentZKMetadata.getCreationTime()).thenReturn(
+ currentTime - MILLISECONDS.convert(2, TimeUnit.HOURS));
+
+ List<PartitionGroupMetadata> partitionGroupMetadataList = new ArrayList<>();
+ int threshold = computer.computeThreshold(
+ streamConfig, committingSegmentDescriptor, committingSegmentZKMetadata,
+ partitionGroupMetadataList, "events3__0__0__20211222T1646Z");
+
+ // (totalDocs / 2) * 1.1
+ // (30000 / 2) * 1.1
+ // 15000 * 1.1
+ assertEquals(threshold, 16_500);
+ }
+
+ @Test
+ public void testSegmentSizeTooSmall() {
+ SegmentFlushThresholdComputer computer = new SegmentFlushThresholdComputer();
+
+ PartitionLevelStreamConfig streamConfig = mock(PartitionLevelStreamConfig.class);
+ when(streamConfig.getFlushThresholdSegmentSizeBytes()).thenReturn(300_0000L);
+
+ CommittingSegmentDescriptor committingSegmentDescriptor = mock(CommittingSegmentDescriptor.class);
+ when(committingSegmentDescriptor.getSegmentSizeBytes()).thenReturn(500_0000L);
+
+ SegmentZKMetadata committingSegmentZKMetadata = mock(SegmentZKMetadata.class);
+ when(committingSegmentZKMetadata.getTotalDocs()).thenReturn(30_000L);
+ when(committingSegmentZKMetadata.getSizeThresholdToFlushSegment()).thenReturn(20_000);
+
+ List<PartitionGroupMetadata> partitionGroupMetadataList = new ArrayList<>();
+ int threshold = computer.computeThreshold(
+ streamConfig, committingSegmentDescriptor, committingSegmentZKMetadata,
+ partitionGroupMetadataList, "events3__0__0__20211222T1646Z");
+
+ // totalDocs / 2
+ // 30000 / 2
+ assertEquals(threshold, 15_000);
+ }
+
+ @Test
+ public void testSegmentSizeTooBig() {
+ SegmentFlushThresholdComputer computer = new SegmentFlushThresholdComputer();
+
+ PartitionLevelStreamConfig streamConfig = mock(PartitionLevelStreamConfig.class);
+ when(streamConfig.getFlushThresholdSegmentSizeBytes()).thenReturn(500_0000L);
+
+ CommittingSegmentDescriptor committingSegmentDescriptor = mock(CommittingSegmentDescriptor.class);
+ when(committingSegmentDescriptor.getSegmentSizeBytes()).thenReturn(200_0000L);
+
+ SegmentZKMetadata committingSegmentZKMetadata = mock(SegmentZKMetadata.class);
+ when(committingSegmentZKMetadata.getTotalDocs()).thenReturn(30_000L);
+ when(committingSegmentZKMetadata.getSizeThresholdToFlushSegment()).thenReturn(20_000);
+
+ List<PartitionGroupMetadata> partitionGroupMetadataList = new ArrayList<>();
+ int threshold = computer.computeThreshold(
+ streamConfig, committingSegmentDescriptor, committingSegmentZKMetadata,
+ partitionGroupMetadataList, "events3__0__0__20211222T1646Z");
+
+ // totalDocs + (totalDocs / 2)
+ // 30000 + (30000 / 2)
+ assertEquals(threshold, 45_000);
+ }
+
+ @Test
+ public void testSegmentSizeJustRight() {
+ SegmentFlushThresholdComputer computer = new SegmentFlushThresholdComputer();
+
+ PartitionLevelStreamConfig streamConfig = mock(PartitionLevelStreamConfig.class);
+ when(streamConfig.getFlushThresholdSegmentSizeBytes()).thenReturn(300_0000L);
+
+ CommittingSegmentDescriptor committingSegmentDescriptor = mock(CommittingSegmentDescriptor.class);
+ when(committingSegmentDescriptor.getSegmentSizeBytes()).thenReturn(250_0000L);
+
+ SegmentZKMetadata committingSegmentZKMetadata = mock(SegmentZKMetadata.class);
+ when(committingSegmentZKMetadata.getTotalDocs()).thenReturn(30_000L);
+ when(committingSegmentZKMetadata.getSizeThresholdToFlushSegment()).thenReturn(20_000);
+
+ List<PartitionGroupMetadata> partitionGroupMetadataList = new ArrayList<>();
+ int threshold = computer.computeThreshold(
+ streamConfig, committingSegmentDescriptor, committingSegmentZKMetadata,
+ partitionGroupMetadataList, "events3__0__0__20211222T1646Z");
+
+ // (totalDocs / segmentSize) * flushThresholdSegmentSize
+ // (30000 / 250000) * 300000
+ assertEquals(threshold, 36_000);
+ }
+
+ @Test
+ public void testNoRows() {
+ SegmentFlushThresholdComputer computer = new SegmentFlushThresholdComputer();
+
+ PartitionLevelStreamConfig streamConfig = mock(PartitionLevelStreamConfig.class);
+ when(streamConfig.getFlushThresholdSegmentSizeBytes()).thenReturn(300_0000L);
+
+ CommittingSegmentDescriptor committingSegmentDescriptor = mock(CommittingSegmentDescriptor.class);
+ when(committingSegmentDescriptor.getSegmentSizeBytes()).thenReturn(250_0000L);
+
+ SegmentZKMetadata committingSegmentZKMetadata = mock(SegmentZKMetadata.class);
+ when(committingSegmentZKMetadata.getTotalDocs()).thenReturn(0L);
+ when(committingSegmentZKMetadata.getSizeThresholdToFlushSegment()).thenReturn(0);
+
+ List<PartitionGroupMetadata> partitionGroupMetadataList = new ArrayList<>();
+ int threshold = computer.computeThreshold(
+ streamConfig, committingSegmentDescriptor, committingSegmentZKMetadata,
+ partitionGroupMetadataList, "events3__0__0__20211222T1646Z");
+
+ // max((totalDocs / segmentSize) * flushThresholdSegmentSize, 10000)
+ // max(0, 10000)
+ assertEquals(threshold, 10_000);
+ }
+
+ @Test
+ public void testAdjustRowsToSizeRatio() {
+ SegmentFlushThresholdComputer computer = new SegmentFlushThresholdComputer();
+
+ PartitionLevelStreamConfig streamConfig = mock(PartitionLevelStreamConfig.class);
+ when(streamConfig.getFlushThresholdSegmentSizeBytes()).thenReturn(300_0000L);
+
+ CommittingSegmentDescriptor committingSegmentDescriptor = mock(CommittingSegmentDescriptor.class);
+ when(committingSegmentDescriptor.getSegmentSizeBytes()).thenReturn(200_000L);
+
+ SegmentZKMetadata committingSegmentZKMetadata = mock(SegmentZKMetadata.class);
+ when(committingSegmentZKMetadata.getTotalDocs()).thenReturn(30_000L, 50_000L);
+ when(committingSegmentZKMetadata.getSizeThresholdToFlushSegment()).thenReturn(60_000);
+
+ List<PartitionGroupMetadata> partitionGroupMetadataList = new ArrayList<>();
+
+ computer.computeThreshold(streamConfig, committingSegmentDescriptor, committingSegmentZKMetadata,
+ partitionGroupMetadataList, "events3__0__0__20211222T1646Z");
+
+ // (totalDocs / segmentSize)
+ // (30000 / 200000)
+ assertEquals(computer.getLatestSegmentRowsToSizeRatio(), 0.15);
+
+ computer.computeThreshold(streamConfig, committingSegmentDescriptor, committingSegmentZKMetadata,
+ partitionGroupMetadataList, "events3__0__0__20211222T1646Z");
+
+ // (0.1 * (totalDocs / segmentSize)) + (0.9 * lastRatio)
+ // (0.1 * (50000 / 200000)) + (0.9 * 0.15)
+ // (0.1 * 0.25) + (0.9 * 0.15)
+ assertEquals(computer.getLatestSegmentRowsToSizeRatio(), 0.16);
+ }
+}
Review comment:
nit: newline at end of file (there's an intellij setting which adds this automatically)
--
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 #7956: Pull out segment flush threshold computation to make it easier to test
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7956:
URL: https://github.com/apache/pinot/pull/7956#issuecomment-1000261734
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7956?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 [#7956](https://codecov.io/gh/apache/pinot/pull/7956?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f4e4f81) into [master](https://codecov.io/gh/apache/pinot/commit/b6eeaf3cf3fbc4592916c5d9db12ae9f9798db6a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b6eeaf3) will **decrease** coverage by `0.01%`.
> The diff coverage is `96.15%`.
[](https://codecov.io/gh/apache/pinot/pull/7956?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 #7956 +/- ##
============================================
- Coverage 71.20% 71.18% -0.02%
+ Complexity 4174 4172 -2
============================================
Files 1593 1594 +1
Lines 82477 82485 +8
Branches 12305 12305
============================================
- Hits 58729 58719 -10
- Misses 19788 19805 +17
- Partials 3960 3961 +1
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `28.99% <0.00%> (-0.08%)` | :arrow_down: |
| integration2 | `27.56% <0.00%> (+0.08%)` | :arrow_up: |
| unittests1 | `68.15% <ø> (-0.04%)` | :arrow_down: |
| unittests2 | `14.33% <96.15%> (+0.02%)` | :arrow_up: |
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/7956?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ealtime/segment/SegmentFlushThresholdComputer.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL3NlZ21lbnQvU2VnbWVudEZsdXNoVGhyZXNob2xkQ29tcHV0ZXIuamF2YQ==) | `95.71% <95.71%> (ø)` | |
| [...segment/SegmentSizeBasedFlushThresholdUpdater.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL3NlZ21lbnQvU2VnbWVudFNpemVCYXNlZEZsdXNoVGhyZXNob2xkVXBkYXRlci5qYXZh) | `100.00% <100.00%> (+28.16%)` | :arrow_up: |
| [...data/manager/realtime/DefaultSegmentCommitter.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvRGVmYXVsdFNlZ21lbnRDb21taXR0ZXIuamF2YQ==) | `0.00% <0.00%> (-80.00%)` | :arrow_down: |
| [...ller/helix/core/minion/TaskTypeMetricsUpdater.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9UYXNrVHlwZU1ldHJpY3NVcGRhdGVyLmphdmE=) | `80.00% <0.00%> (-20.00%)` | :arrow_down: |
| [...er/api/resources/LLCSegmentCompletionHandlers.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL0xMQ1NlZ21lbnRDb21wbGV0aW9uSGFuZGxlcnMuamF2YQ==) | `43.56% <0.00%> (-18.82%)` | :arrow_down: |
| [...nt/local/startree/v2/store/StarTreeDataSource.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9zdG9yZS9TdGFyVHJlZURhdGFTb3VyY2UuamF2YQ==) | `40.00% <0.00%> (-13.34%)` | :arrow_down: |
| [...data/manager/realtime/SegmentCommitterFactory.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvU2VnbWVudENvbW1pdHRlckZhY3RvcnkuamF2YQ==) | `88.23% <0.00%> (-11.77%)` | :arrow_down: |
| [...altime/ServerSegmentCompletionProtocolHandler.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VydmVyL3JlYWx0aW1lL1NlcnZlclNlZ21lbnRDb21wbGV0aW9uUHJvdG9jb2xIYW5kbGVyLmphdmE=) | `51.42% <0.00%> (-6.67%)` | :arrow_down: |
| [...ot/segment/local/startree/OffHeapStarTreeNode.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS9PZmZIZWFwU3RhclRyZWVOb2RlLmphdmE=) | `72.22% <0.00%> (-5.56%)` | :arrow_down: |
| [...lix/core/realtime/PinotRealtimeSegmentManager.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90UmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh) | `77.48% <0.00%> (-5.24%)` | :arrow_down: |
| ... and [25 more](https://codecov.io/gh/apache/pinot/pull/7956/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/7956?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/7956?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 [b6eeaf3...f4e4f81](https://codecov.io/gh/apache/pinot/pull/7956?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 #7956: Pull out segment flush threshold computation to make it easier to test
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #7956:
URL: https://github.com/apache/pinot/pull/7956#discussion_r774500065
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/segment/Clock.java
##########
@@ -0,0 +1,23 @@
+/**
+ * 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.controller.helix.core.realtime.segment;
+
+interface Clock {
Review comment:
We can use `java.time.Clock` which has the `millis()` method instead. It has a system clock at any offset and also allows use of a fixed clock for testing.
--
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] mneedham commented on a change in pull request #7956: Pull out segment flush threshold computation to make it easier to test
Posted by GitBox <gi...@apache.org>.
mneedham commented on a change in pull request #7956:
URL: https://github.com/apache/pinot/pull/7956#discussion_r776511660
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/segment/SegmentFlushThresholdComputer.java
##########
@@ -0,0 +1,182 @@
+/**
+ * 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.controller.helix.core.realtime.segment;
+
+import java.time.Clock;
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.metadata.segment.SegmentZKMetadata;
+import org.apache.pinot.common.utils.LLCSegmentName;
+import org.apache.pinot.spi.stream.PartitionGroupMetadata;
+import org.apache.pinot.spi.stream.PartitionLevelStreamConfig;
+import org.apache.pinot.spi.utils.TimeUtils;
+
+public class SegmentFlushThresholdComputer {
+ public static final int MINIMUM_NUM_ROWS_THRESHOLD = 10_000;
Review comment:
True, have updated
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/segment/SegmentFlushThresholdComputer.java
##########
@@ -0,0 +1,182 @@
+/**
+ * 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.controller.helix.core.realtime.segment;
+
+import java.time.Clock;
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.metadata.segment.SegmentZKMetadata;
+import org.apache.pinot.common.utils.LLCSegmentName;
+import org.apache.pinot.spi.stream.PartitionGroupMetadata;
+import org.apache.pinot.spi.stream.PartitionLevelStreamConfig;
+import org.apache.pinot.spi.utils.TimeUtils;
+
+public class SegmentFlushThresholdComputer {
+ public static final int MINIMUM_NUM_ROWS_THRESHOLD = 10_000;
+ static final double CURRENT_SEGMENT_RATIO_WEIGHT = 0.1;
+ static final double PREVIOUS_SEGMENT_RATIO_WEIGHT = 0.9;
+ static final double ROWS_MULTIPLIER_WHEN_TIME_THRESHOLD_HIT = 1.1;
+
+ // num rows to segment size ratio of last committed segment for this table
+ private double _latestSegmentRowsToSizeRatio;
+ private final Clock _clock;
+
+ public SegmentFlushThresholdComputer() {
+ this(Clock.systemUTC(), 0);
+ }
+
+ public SegmentFlushThresholdComputer(Clock clock, double latestSegmentRowsToSizeRatio) {
Review comment:
done!
--
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] mneedham commented on a change in pull request #7956: Pull out segment flush threshold computation to make it easier to test
Posted by GitBox <gi...@apache.org>.
mneedham commented on a change in pull request #7956:
URL: https://github.com/apache/pinot/pull/7956#discussion_r774503478
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/segment/Clock.java
##########
@@ -0,0 +1,23 @@
+/**
+ * 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.controller.helix.core.realtime.segment;
+
+interface Clock {
Review comment:
Oh nice! I didn't know you could do that, will update.
--
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 #7956: Pull out segment flush threshold computation to make it easier to test
Posted by GitBox <gi...@apache.org>.
richardstartin commented on pull request #7956:
URL: https://github.com/apache/pinot/pull/7956#issuecomment-1000231985
Injectable clocks are essential if you want to test time-dependent behaviour, great initiative 👍🏻
--
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] npawar merged pull request #7956: Pull out segment flush threshold computation to make it easier to test
Posted by GitBox <gi...@apache.org>.
npawar merged pull request #7956:
URL: https://github.com/apache/pinot/pull/7956
--
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] mneedham commented on a change in pull request #7956: Pull out segment flush threshold computation to make it easier to test
Posted by GitBox <gi...@apache.org>.
mneedham commented on a change in pull request #7956:
URL: https://github.com/apache/pinot/pull/7956#discussion_r774518505
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/segment/SegmentFlushThresholdComputer.java
##########
@@ -0,0 +1,184 @@
+/**
+ * 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.controller.helix.core.realtime.segment;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.metadata.segment.SegmentZKMetadata;
+import org.apache.pinot.common.utils.LLCSegmentName;
+import org.apache.pinot.spi.stream.PartitionGroupMetadata;
+import org.apache.pinot.spi.stream.PartitionLevelStreamConfig;
+import org.apache.pinot.spi.utils.TimeUtils;
+
+import static org.apache.pinot.controller.helix.core.realtime.segment.SegmentSizeBasedFlushThresholdUpdater.LOGGER;
+
+
+public class SegmentFlushThresholdComputer {
+ public static final int MINIMUM_NUM_ROWS_THRESHOLD = 10_000;
+ static final double CURRENT_SEGMENT_RATIO_WEIGHT = 0.1;
+ static final double PREVIOUS_SEGMENT_RATIO_WEIGHT = 0.9;
+ static final double ROWS_MULTIPLIER_WHEN_TIME_THRESHOLD_HIT = 1.1;
+
+ // num rows to segment size ratio of last committed segment for this table
+ private double _latestSegmentRowsToSizeRatio;
+ private final Clock _clock;
+
+ public SegmentFlushThresholdComputer() {
+ this(new SystemClock(), 0);
+ }
+
+ public SegmentFlushThresholdComputer(Clock clock, double latestSegmentRowsToSizeRatio) {
+ this._clock = clock;
+ this._latestSegmentRowsToSizeRatio = latestSegmentRowsToSizeRatio;
+ }
+
+ public SegmentFlushThresholdComputer(Clock clock) {
+ this(clock, 0);
+ }
+
+ double getLatestSegmentRowsToSizeRatio() {
+ return _latestSegmentRowsToSizeRatio;
+ }
+
+ public int computeThreshold(PartitionLevelStreamConfig streamConfig,
+ CommittingSegmentDescriptor committingSegmentDescriptor,
+ @Nullable SegmentZKMetadata committingSegmentZKMetadata,
+ List<PartitionGroupMetadata> partitionGroupMetadataList, String newSegmentName) {
+ final long desiredSegmentSizeBytes = streamConfig.getFlushThresholdSegmentSizeBytes();
+ final long optimalSegmentSizeBytesMin = desiredSegmentSizeBytes / 2;
+ final double optimalSegmentSizeBytesMax = desiredSegmentSizeBytes * 1.5;
+
+ if (committingSegmentZKMetadata == null) { // first segment of the partition, hence committing segment is null
+ if (_latestSegmentRowsToSizeRatio > 0) { // new partition group added case
+ long targetSegmentNumRows = (long) (desiredSegmentSizeBytes * _latestSegmentRowsToSizeRatio);
+ targetSegmentNumRows = capNumRowsIfOverflow(targetSegmentNumRows);
+ LOGGER.info(
+ "Committing segment zk metadata is not available, using prev ratio {}, setting rows threshold for {} as {}",
+ _latestSegmentRowsToSizeRatio, newSegmentName, targetSegmentNumRows);
+ return (int) targetSegmentNumRows;
+ } else {
+ final int autotuneInitialRows = streamConfig.getFlushAutotuneInitialRows();
+ LOGGER.info(
+ "Committing segment zk metadata is not available, setting threshold for {} as {}", newSegmentName,
+ autotuneInitialRows);
+ return autotuneInitialRows;
+ }
+ }
+
+ final long committingSegmentSizeBytes = committingSegmentDescriptor.getSegmentSizeBytes();
+ if (committingSegmentSizeBytes <= 0) { // repair segment case
+ final int targetNumRows = committingSegmentZKMetadata.getSizeThresholdToFlushSegment();
+ LOGGER.info(
+ "Committing segment size is not available, setting thresholds from previous segment for {} as {}",
+ newSegmentName, targetNumRows);
+ return targetNumRows;
+ }
+
+ final long timeConsumed = _clock.currentTimeMillis() - committingSegmentZKMetadata.getCreationTime();
+ final long numRowsConsumed = committingSegmentZKMetadata.getTotalDocs();
+ final int numRowsThreshold = committingSegmentZKMetadata.getSizeThresholdToFlushSegment();
+ LOGGER.info(
+ "{}: Data from committing segment: Time {} numRows {} threshold {} segmentSize(bytes) {}",
+ newSegmentName, TimeUtils.convertMillisToPeriod(timeConsumed), numRowsConsumed, numRowsThreshold,
+ committingSegmentSizeBytes);
+
+ double currentRatio = (double) numRowsConsumed / committingSegmentSizeBytes;
+ // Compute segment size to rows ratio only from the lowest available partition id.
+ // If we consider all partitions then it is likely that we will assign a much higher weight to the most
+ // recent trend in the table (since it is usually true that all partitions of the same table follow more or
+ // less same characteristics at any one point in time).
+ // However, when we start a new table or change controller mastership, we can have any partition completing first.
+ // It is best to learn the ratio as quickly as we can, so we allow any partition to supply the value.
+ int smallestAvailablePartitionGroupId =
+ partitionGroupMetadataList.stream().mapToInt(PartitionGroupMetadata::getPartitionGroupId).min()
+ .orElseGet(() -> 0);
Review comment:
Makes sense, done.
--
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] mcvsubbu removed a comment on pull request #7956: Pull out segment flush threshold computation to make it easier to test
Posted by GitBox <gi...@apache.org>.
mcvsubbu removed a comment on pull request #7956:
URL: https://github.com/apache/pinot/pull/7956#issuecomment-1000429101
Any reason to change the name? It _is_ size based threshold updater. FYI, we have time based and row based as well.
--
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 #7956: Pull out segment flush threshold computation to make it easier to test
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7956:
URL: https://github.com/apache/pinot/pull/7956#issuecomment-1000261734
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7956?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 [#7956](https://codecov.io/gh/apache/pinot/pull/7956?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f4e4f81) into [master](https://codecov.io/gh/apache/pinot/commit/b6eeaf3cf3fbc4592916c5d9db12ae9f9798db6a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b6eeaf3) will **decrease** coverage by `56.86%`.
> The diff coverage is `96.15%`.
[](https://codecov.io/gh/apache/pinot/pull/7956?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 #7956 +/- ##
=============================================
- Coverage 71.20% 14.33% -56.87%
+ Complexity 4174 80 -4094
=============================================
Files 1593 1549 -44
Lines 82477 80614 -1863
Branches 12305 12103 -202
=============================================
- Hits 58729 11560 -47169
- Misses 19788 68197 +48409
+ Partials 3960 857 -3103
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.33% <96.15%> (+0.02%)` | :arrow_up: |
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/7956?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ealtime/segment/SegmentFlushThresholdComputer.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL3NlZ21lbnQvU2VnbWVudEZsdXNoVGhyZXNob2xkQ29tcHV0ZXIuamF2YQ==) | `95.71% <95.71%> (ø)` | |
| [...segment/SegmentSizeBasedFlushThresholdUpdater.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL3NlZ21lbnQvU2VnbWVudFNpemVCYXNlZEZsdXNoVGhyZXNob2xkVXBkYXRlci5qYXZh) | `100.00% <100.00%> (+28.16%)` | :arrow_up: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/BigDecimalUtils.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQmlnRGVjaW1hbFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/common/tier/TierFactory.java](https://codecov.io/gh/apache/pinot/pull/7956/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1263 more](https://codecov.io/gh/apache/pinot/pull/7956/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/7956?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/7956?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 [b6eeaf3...f4e4f81](https://codecov.io/gh/apache/pinot/pull/7956?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] mcvsubbu commented on pull request #7956: Pull out segment flush threshold computation to make it easier to test
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on pull request #7956:
URL: https://github.com/apache/pinot/pull/7956#issuecomment-1000429101
Any reason to change the name? It _is_ size based threshold updater. FYI, we have time based and row based as well.
--
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