[GitHub] [incubator-pinot] adriancole commented on pull request #6039: WIP: ServiceManager ADD_TABLE role
adriancole commented on pull request #6039: URL: https://github.com/apache/incubator-pinot/pull/6039#issuecomment-706963614 nagging https://github.com/apache/incubator-pinot/issues/5975 as ran into it again. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] adriancole commented on pull request #6039: WIP: ServiceManager ADD_TABLE role
adriancole commented on pull request #6039: URL: https://github.com/apache/incubator-pinot/pull/6039#issuecomment-707063779 functionally, this works if the "add table" commands are done last (because effectively, you end up needing to wait for everything anyway). Verified here https://github.com/hypertrace/pinot/pull/48 It may seem not that much better if done in java vs shell scripts, especially as everything has to be ready first. However, java is easier to test, there's less network problems, and a chance to be considered a bootstrap operation (crash the process if it fails, participate in overall health) I understand that there's some debate about where to stuff the add table command. I also understand there is some angst about this temporarily using the "service" role. This was to perform the least change as literally ServiceManager is designed to work on services. I don't really care if this is called a service or not. However, there's a lack of an alternative also. This brings to the next action. I'm hoping someone maybe @kishoreg can find an agreeable place to house this logic. Once those debates are over, I can take a look at backfilling tests etc in that context. Of course, if someone wants this code to do on own, they can do that also. NEXT STEP: knowing a role ADD_TABLE won't fly, await an alternative. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #6131: F master fix
fx19880617 commented on a change in pull request #6131: URL: https://github.com/apache/incubator-pinot/pull/6131#discussion_r503389188 ## File path: Dockerfile.jdk14 ## @@ -0,0 +1,74 @@ +# Review comment: should you put this under thirdeye/docker ? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] venkatvghub closed pull request #6130: F master fix
venkatvghub closed pull request #6130: URL: https://github.com/apache/incubator-pinot/pull/6130 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] venkatvghub commented on pull request #6130: F master fix
venkatvghub commented on pull request #6130: URL: https://github.com/apache/incubator-pinot/pull/6130#issuecomment-707011270 Invalid This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] venkatvghub opened a new pull request #6130: F master fix
venkatvghub opened a new pull request #6130: URL: https://github.com/apache/incubator-pinot/pull/6130 ## Description - Merges from upstream master - Adds support for drone, JDK 14 and Mysql 8 ## 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. 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
[GitHub] [incubator-pinot] venkatvghub opened a new pull request #6131: F master fix
venkatvghub opened a new pull request #6131: URL: https://github.com/apache/incubator-pinot/pull/6131 ## Description - Adds drone.yml - Adds support for java 11+ - Updates mysql connector to 8+ - Adds custom Docker file for above changes This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io commented on pull request #6133: Remove flaky test from SegmentReducerTest
codecov-io commented on pull request #6133: URL: https://github.com/apache/incubator-pinot/pull/6133#issuecomment-707295123 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6133?src=pr=h1) Report > Merging [#6133](https://codecov.io/gh/apache/incubator-pinot/pull/6133?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) will **decrease** coverage by `21.00%`. > The diff coverage is `48.75%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/6133?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#6133 +/- ## === - Coverage 66.44% 45.44% -21.01% === Files1075 1223 +148 Lines 5477357744 +2971 Branches 8168 8522 +354 === - Hits3639626239-10157 - Misses 1570029373+13673 + Partials 2677 2132 -545 ``` | Flag | Coverage Δ | | |---|---|---| | #integration | `45.44% <48.75%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6133?src=pr=tree) | Coverage Δ | | |---|---|---| | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | | | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: | | [...org/apache/pinot/broker/queryquota/HitCounter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9IaXRDb3VudGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: | | [...che/pinot/broker/queryquota/MaxHitRateTracker.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9NYXhIaXRSYXRlVHJhY2tlci5qYXZh) | `0.00% <0.00%> (ø)` | | | [...ache/pinot/broker/queryquota/QueryQuotaEntity.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9RdWVyeVF1b3RhRW50aXR5LmphdmE=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: | | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `26.66% <0.00%> (-30.48%)` | :arrow_down: | | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `22.22% <0.00%> (-26.62%)` | :arrow_down: | | [.../org/apache/pinot/client/ResultTableResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFRhYmxlUmVzdWx0U2V0LmphdmE=) | `24.00% <0.00%> (-10.29%)` | :arrow_down: | | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `64.28% <ø> (-8.89%)` | :arrow_down: | | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: | | ... and [1204 more](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6133?src=pr=continue). > **Legend** - [Click
[GitHub] [incubator-pinot] sajjad-moradi commented on a change in pull request #6129: Issue #4854 Framework for adding compatibility tests
sajjad-moradi commented on a change in pull request #6129: URL: https://github.com/apache/incubator-pinot/pull/6129#discussion_r503461466 ## File path: compatibility-verifier/compCheck.sh ## @@ -157,20 +181,27 @@ fi # Setup initial cluster with olderCommit and do rolling upgrade startServices "$oldTargetDir" -sleep 20 +#$COMPAT_TESTER pre-controller-upgrade.yaml Review comment: I'm not sure why 20 seconds sleep was chosen in the first place, but does replacing it with running compat_tester mean the tests are going to take 20 seconds? Also maybe it reads better if $COMPAT_TESTER is replaced with a function call, something like runCompatiblityTests, which is more consistent with startService, startService, ... here. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu opened a new issue #6132: Intermittent failures in SegmentReducerTest
mcvsubbu opened a new issue #6132: URL: https://github.com/apache/incubator-pinot/issues/6132 We see this occasionally: Tests run: 7, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 1.924 sec <<< FAILURE! - in org.apache.pinot.core.segment.processing.framework.SegmentReducerTest segmentReducerTest(org.apache.pinot.core.segment.processing.framework.SegmentReducerTest) Time elapsed: 0.096 sec segmentReducerTest(org.apache.pinot.core.segment.processing.framework.SegmentReducerTest) Time elapsed: 0.005 sec segmentReducerTest(org.apache.pinot.core.segment.processing.framework.SegmentReducerTest) Time elapsed: 0.006 sec segmentReducerTest(org.apache.pinot.core.segment.processing.framework.SegmentReducerTest) Time elapsed: 0.004 sec segmentReducerTest(org.apache.pinot.core.segment.processing.framework.SegmentReducerTest) Time elapsed: 0.011 sec <<< FAILURE! java.lang.AssertionError: expected [3000] but found [4000] at org.testng.Assert.fail(Assert.java:93) at org.testng.Assert.failNotEquals(Assert.java:512) at org.testng.Assert.assertEqualsImpl(Assert.java:134) at org.testng.Assert.assertEquals(Assert.java:115) at org.testng.Assert.assertEquals(Assert.java:178) at org.apache.pinot.core.segment.processing.framework.SegmentReducerTest.segmentReducerTest(SegmentReducerTest.java:143) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:108) at org.testng.internal.Invoker.invokeMethod(Invoker.java:661) at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:869) at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1193) at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:126) at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:109) at org.testng.TestRunner.privateRun(TestRunner.java:744) at org.testng.TestRunner.run(TestRunner.java:602) at org.testng.SuiteRunner.runTest(SuiteRunner.java:380) at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:375) at org.testng.SuiteRunner.privateRun(SuiteRunner.java:340) at org.testng.SuiteRunner.run(SuiteRunner.java:289) at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52) at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86) at org.testng.TestNG.runSuitesSequentially(TestNG.java:1301) at org.testng.TestNG.runSuitesLocally(TestNG.java:1226) at org.testng.TestNG.runSuites(TestNG.java:1144) at org.testng.TestNG.run(TestNG.java:1115) at org.apache.maven.surefire.testng.TestNGExecutor.run(TestNGExecutor.java:132) at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.executeSingleClass(TestNGDirectoryTestSuite.java:134) at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.execute(TestNGDirectoryTestSuite.java:118) at org.apache.maven.surefire.testng.TestNGProvider.invoke(TestNGProvider.java:146) at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:286) at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:240) at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:121) segmentReducerTest(org.apache.pinot.core.segment.processing.framework.SegmentReducerTest) Time elapsed: 0.006 sec segmentReducerTest(org.apache.pinot.core.segment.processing.framework.SegmentReducerTest) Time elapsed: 0.003 sec This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] 01/01: Adding a flag in PinotServiceManager to allow health check includes all components health check
This is an automated email from the ASF dual-hosted git repository.
xiangfu pushed a commit to branch pinotSM_flag_for_health_check
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
commit 01419655e077675768a9e641258a8313be767b13
Author: Xiang Fu
AuthorDate: Mon Oct 12 12:51:04 2020 -0700
Adding a flag in PinotServiceManager to allow health check includes all
components health check
---
.../tools/admin/command/StartServiceManagerCommand.java | 12 +++-
.../apache/pinot/tools/service/PinotServiceManager.java | 16 +++-
.../api/resources/PinotServiceManagerHealthCheck.java| 15 +++
3 files changed, 37 insertions(+), 6 deletions(-)
diff --git
a/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/StartServiceManagerCommand.java
b/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/StartServiceManagerCommand.java
index caa1ca0..a470efd 100644
---
a/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/StartServiceManagerCommand.java
+++
b/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/StartServiceManagerCommand.java
@@ -69,6 +69,8 @@ public class StartServiceManagerCommand extends
AbstractBaseAdminCommand impleme
private String _clusterName = DEFAULT_CLUSTER_NAME;
@Option(name = "-port", required = true, metaVar = "", usage = "Pinot
service manager admin port, -1 means disable, 0 means a random available port.")
private int _port;
+ @Option(name = "-healthCheckAllComponents", metaVar = "", usage =
"Pinot service manager health check returns the all components health check.
The health check returns OK when all the components are returning OK.")
+ private boolean _healthCheckAllComponents;
@Option(name = "-bootstrapConfigPaths", handler =
StringArrayOptionHandler.class, required = false, usage = "A list of Pinot
service config file paths. Each config file requires an extra config:
'pinot.service.role' to indicate which service to start.", forbids =
{"-bootstrapServices"})
private String[] _bootstrapConfigPaths;
@Option(name = "-bootstrapServices", handler =
StringArrayOptionHandler.class, required = false, usage = "A list of Pinot
service roles to start with default config. E.g. CONTROLLER/BROKER/SERVER",
forbids = {"-bootstrapConfigPaths"})
@@ -195,7 +197,7 @@ public class StartServiceManagerCommand extends
AbstractBaseAdminCommand impleme
}
private String startServiceManager() {
-_pinotServiceManager = new PinotServiceManager(_zkAddress, _clusterName,
_port);
+_pinotServiceManager = new PinotServiceManager(_zkAddress, _clusterName,
_port, _healthCheckAllComponents);
_pinotServiceManager.start();
return _pinotServiceManager.getInstanceId();
}
@@ -303,4 +305,12 @@ public class StartServiceManagerCommand extends
AbstractBaseAdminCommand impleme
_bootstrapConfigurations.add(new SimpleImmutableEntry<>(role, config));
return this;
}
+
+ public boolean isHealthCheckAllComponents() {
+return _healthCheckAllComponents;
+ }
+
+ public void setHealthCheckAllComponents(boolean healthCheckAllComponents) {
+_healthCheckAllComponents = healthCheckAllComponents;
+ }
}
diff --git
a/pinot-tools/src/main/java/org/apache/pinot/tools/service/PinotServiceManager.java
b/pinot-tools/src/main/java/org/apache/pinot/tools/service/PinotServiceManager.java
index 63fa3ea..ed8305f 100644
---
a/pinot-tools/src/main/java/org/apache/pinot/tools/service/PinotServiceManager.java
+++
b/pinot-tools/src/main/java/org/apache/pinot/tools/service/PinotServiceManager.java
@@ -54,18 +54,19 @@ public class PinotServiceManager {
private final String _clusterName;
private final int _port;
private final String _instanceId;
+ private final boolean _healthCheckAllComponents;
private PinotServiceManagerAdminApiApplication
_pinotServiceManagerAdminApplication;
private boolean _isStarted = false;
public PinotServiceManager(String zkAddress, String clusterName) {
-this(zkAddress, clusterName, 0);
+this(zkAddress, clusterName, 0, false);
}
- public PinotServiceManager(String zkAddress, String clusterName, int port) {
-this(zkAddress, clusterName, null, port);
+ public PinotServiceManager(String zkAddress, String clusterName, int port,
boolean healthCheckAllComponents) {
+this(zkAddress, clusterName, null, port, healthCheckAllComponents);
}
- public PinotServiceManager(String zkAddress, String clusterName, String
hostname, int port) {
+ public PinotServiceManager(String zkAddress, String clusterName, String
hostname, int port, boolean healthCheckAllComponents) {
_zkAddress = zkAddress;
_clusterName = clusterName;
if (port == 0) {
@@ -76,10 +77,11 @@ public class PinotServiceManager {
hostname = NetUtil.getHostnameOrAddress();
}
_instanceId = String.format("ServiceManager_%s_%d", hostname, port);
+_healthCheckAllComponents = healthCheckAllComponents;
}
[incubator-pinot] branch pinotSM_flag_for_health_check created (now 0141965)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch pinotSM_flag_for_health_check in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. at 0141965 Adding a flag in PinotServiceManager to allow health check includes all components health check This branch includes the following new commits: new 0141965 Adding a flag in PinotServiceManager to allow health check includes all components health check The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] npawar opened a new pull request #6133: Remove flaky test from SegmentReducerTest
npawar opened a new pull request #6133: URL: https://github.com/apache/incubator-pinot/pull/6133 It seems the order returned in File#listFiles() is not guaranteed. The results of this test change depending on the order of the files returned in File#listFiles(). We received some complaints of this test failing. Removing this test, because the test cases (ROLLUP aggregation, custom numRecordsPerSegment) are already covered in other test cases in the same test file. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 opened a new pull request #6134: Adding a flag in PinotServiceManager to allow health check includes all components health check
fx19880617 opened a new pull request #6134: URL: https://github.com/apache/incubator-pinot/pull/6134 ## Description - Adding a flag in PinotServiceManager to allow health check includes all components health check This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] sajjad-moradi commented on a change in pull request #6129: Issue #4854 Framework for adding compatibility tests
sajjad-moradi commented on a change in pull request #6129: URL: https://github.com/apache/incubator-pinot/pull/6129#discussion_r503462097 ## File path: compatibility-verifier/compCheck.sh ## @@ -157,20 +181,27 @@ fi # Setup initial cluster with olderCommit and do rolling upgrade startServices "$oldTargetDir" -sleep 20 +#$COMPAT_TESTER pre-controller-upgrade.yaml stopService controller "$oldTargetDir" startService controller "$newTargetDir" +#$COMPAT_TESTER pre-broker-upgrade.yaml stopService broker "$oldTargetDir" startService broker "$newTargetDir" +#$COMPAT_TESTER pre-server-upgrade.yaml stopService server "$oldTargetDir" startService server "$newTargetDir" -sleep 20 +#$COMPAT_TESTER post-server-upgrade.yaml + +# Upgrade complated, now do a rollback stopService controller "$newTargetDir" startService controller "$oldTargetDir" +#$COMPAT_TESTER post-server-rollback.yaml stopService broker "$newTargetDir" startService broker "$oldTargetDir" +#$COMPAT_TESTER post-broker-rollback.yaml stopService server "$newTargetDir" startService server "$oldTargetDir" +#$COMPAT_TESTER post-controller-rollback.yaml sleep 20 Review comment: If you decide to have(or remove) previous `sleep`s, please do the same for this one 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. 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
[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6133: Remove flaky test from SegmentReducerTest
codecov-io edited a comment on pull request #6133: URL: https://github.com/apache/incubator-pinot/pull/6133#issuecomment-707295123 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6133?src=pr=h1) Report > Merging [#6133](https://codecov.io/gh/apache/incubator-pinot/pull/6133?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) will **increase** coverage by `6.41%`. > The diff coverage is `59.69%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/6133?src=pr=tree) ```diff @@Coverage Diff @@ ## master#6133 +/- ## == + Coverage 66.44% 72.86% +6.41% == Files1075 1223 +148 Lines 5477357744+2971 Branches 8168 8522 +354 == + Hits3639642077+5681 + Misses 1570012910-2790 - Partials 2677 2757 +80 ``` | Flag | Coverage Δ | | |---|---|---| | #integration | `45.44% <48.75%> (?)` | | | #unittests | `64.01% <38.00%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6133?src=pr=tree) | Coverage Δ | | |---|---|---| | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | | | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: | | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `53.33% <0.00%> (-3.81%)` | :arrow_down: | | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `44.44% <0.00%> (-4.40%)` | :arrow_down: | | [.../org/apache/pinot/client/ResultTableResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFRhYmxlUmVzdWx0U2V0LmphdmE=) | `24.00% <0.00%> (-10.29%)` | :arrow_down: | | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `78.57% <ø> (+5.40%)` | :arrow_up: | | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: | | [...pinot/common/function/AggregationFunctionType.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vQWdncmVnYXRpb25GdW5jdGlvblR5cGUuamF2YQ==) | `98.27% <ø> (-1.73%)` | :arrow_down: | | [.../pinot/common/function/DateTimePatternHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRGF0ZVRpbWVQYXR0ZXJuSGFuZGxlci5qYXZh) | `83.33% <ø> (ø)` | | | [...ot/common/function/FunctionDefinitionRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRnVuY3Rpb25EZWZpbml0aW9uUmVnaXN0cnkuamF2YQ==) | `88.88% <ø> (+44.44%)` | :arrow_up: | | ... and [973 more](https://codecov.io/gh/apache/incubator-pinot/pull/6133/diff?src=pr=tree-more) | | -- [Continue to review full report at
[GitHub] [incubator-pinot] Jackie-Jiang commented on pull request #6022: Add IN_SUBQUERY support
Jackie-Jiang commented on pull request #6022: URL: https://github.com/apache/incubator-pinot/pull/6022#issuecomment-707302712 > Why do we need a transform function to support subquery? Can we use the standard SQL syntax of providing the subquery inside parentheses to clearly distinguish between outer and inner queries. @siddharthteotia Good question. Currently Pinot does not support nested query or join (both query parser and engine), so we use transform function as a work-around to achieve the same semantic. In the future we might want to provide the native nested query and join support, or provide a translation layer where such queries can be rewritten into the format in this PR. The scope of supporting nested query and join is way larger than this PR, so right now we force users to use the transform function for the id_set subquery. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] deemoliu commented on a change in pull request #6112: add max length support in schema builder
deemoliu commented on a change in pull request #6112:
URL: https://github.com/apache/incubator-pinot/pull/6112#discussion_r503563957
##
File path:
pinot-common/src/test/java/org/apache/pinot/common/data/FieldSpecTest.java
##
@@ -80,6 +80,18 @@ public void testFieldSpec() {
Assert.assertEquals(fieldSpec1.hashCode(), fieldSpec2.hashCode());
Assert.assertEquals(fieldSpec1.getDefaultNullValue(), "false");
+// Single-value boolean type dimension field with max length and default
null value.
+fieldSpec1 = new DimensionFieldSpec();
+fieldSpec1.setName("svDimension");
+fieldSpec1.setDataType(BOOLEAN);
+fieldSpec1.setDefaultNullValue(false);
+fieldSpec1.setMaxLength(2);
Review comment:
I see. Then I will remove the code related to "metrics with max length"
(since maxlength ony applied to String and metrics columns cannot be string
type) and only keep "dimension with max length".
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6125: Make transform functions support underscore in the query functions
Jackie-Jiang commented on a change in pull request #6125:
URL: https://github.com/apache/incubator-pinot/pull/6125#discussion_r503583619
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/TransformFunctionFactory.java
##
@@ -62,73 +63,73 @@ private TransformFunctionFactory() {
new HashMap>() {
{
// NOTE: add all built-in transform functions here
- put(TransformFunctionType.ADD.getName().toLowerCase(),
AdditionTransformFunction.class);
- put(TransformFunctionType.SUB.getName().toLowerCase(),
SubtractionTransformFunction.class);
- put(TransformFunctionType.MULT.getName().toLowerCase(),
MultiplicationTransformFunction.class);
- put(TransformFunctionType.DIV.getName().toLowerCase(),
DivisionTransformFunction.class);
- put(TransformFunctionType.MOD.getName().toLowerCase(),
ModuloTransformFunction.class);
-
- put(TransformFunctionType.PLUS.getName().toLowerCase(),
AdditionTransformFunction.class);
- put(TransformFunctionType.MINUS.getName().toLowerCase(),
SubtractionTransformFunction.class);
- put(TransformFunctionType.TIMES.getName().toLowerCase(),
MultiplicationTransformFunction.class);
- put(TransformFunctionType.DIVIDE.getName().toLowerCase(),
DivisionTransformFunction.class);
-
- put(TransformFunctionType.ABS.getName().toLowerCase(),
AbsTransformFunction.class);
- put(TransformFunctionType.CEIL.getName().toLowerCase(),
CeilTransformFunction.class);
- put(TransformFunctionType.EXP.getName().toLowerCase(),
ExpTransformFunction.class);
- put(TransformFunctionType.FLOOR.getName().toLowerCase(),
FloorTransformFunction.class);
- put(TransformFunctionType.LN.getName().toLowerCase(),
LnTransformFunction.class);
- put(TransformFunctionType.SQRT.getName().toLowerCase(),
SqrtTransformFunction.class);
-
- put(TransformFunctionType.CAST.getName().toLowerCase(),
CastTransformFunction.class);
- put(TransformFunctionType.JSONEXTRACTSCALAR.getName().toLowerCase(),
+ put(canonicalize(TransformFunctionType.ADD.getName().toLowerCase()),
AdditionTransformFunction.class);
Review comment:
You can remove the `toLowerCase()` here
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #6112: add max length support in schema builder
chenboat commented on a change in pull request #6112:
URL: https://github.com/apache/incubator-pinot/pull/6112#discussion_r503563222
##
File path:
pinot-common/src/test/java/org/apache/pinot/common/data/FieldSpecTest.java
##
@@ -80,6 +80,18 @@ public void testFieldSpec() {
Assert.assertEquals(fieldSpec1.hashCode(), fieldSpec2.hashCode());
Assert.assertEquals(fieldSpec1.getDefaultNullValue(), "false");
+// Single-value boolean type dimension field with max length and default
null value.
+fieldSpec1 = new DimensionFieldSpec();
+fieldSpec1.setName("svDimension");
+fieldSpec1.setDataType(BOOLEAN);
+fieldSpec1.setDefaultNullValue(false);
+fieldSpec1.setMaxLength(2);
Review comment:
It looks like the doc is not in sync with the code. The code is usually
the source of truth. For this PR, I think the update to METRICS column can be
skipped.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6113: Adding the upsert support to real-time ingestion and query
mcvsubbu commented on a change in pull request #6113:
URL: https://github.com/apache/incubator-pinot/pull/6113#discussion_r503515858
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java
##
@@ -132,6 +146,19 @@ protected void doInit() {
String consumerDirPath = getConsumerDir();
File consumerDir = new File(consumerDirPath);
+TableConfig tableConfig =
ZKMetadataProvider.getTableConfig(_propertyStore, _tableNameWithType);
Review comment:
tableconfig is available at the time TableDataManager is created. Can
you use that instead of fetching it again here?
Also, TableDataManager is not recreaated if table config changes. How do you
plan to address tha?
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java
##
@@ -1165,6 +1172,14 @@ public
LLRealtimeSegmentDataManager(RealtimeSegmentZKMetadata segmentZKMetadata,
Set textIndexColumns = indexLoadingConfig.getTextIndexColumns();
_textIndexColumns = new ArrayList<>(textIndexColumns);
+PartitionUpsertMetadataManager partitionUpsertMetadataManager = null;
+UpsertConfig.Mode upsertMode = _tableConfig.getUpsertMode();
+if (_upsertMetadataTableManager != null && upsertMode !=
UpsertConfig.Mode.NONE) {
+ int partitionId = new LLCSegmentName(_segmentNameStr).getPartitionId();
Review comment:
Please ise `_streamPartitionId` member variable
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/indexsegment/mutable/MutableSegmentImpl.java
##
@@ -453,6 +465,49 @@ public boolean index(GenericRow row, @Nullable RowMetadata
rowMetadata) {
return canTakeMore;
}
+ private boolean isUpsertEnabled() {
+return _upsertMode != null && _upsertMode != UpsertConfig.Mode.NONE;
+ }
+
+ private void handleUpsert(GenericRow row, int docId) {
+// below are upsert operations
+PrimaryKey primaryKey = row.getPrimaryKey(_schema.getPrimaryKeyColumns());
+Object timeValue = row.getValue(_timeColumnName);
+Preconditions.checkArgument(timeValue instanceof Comparable, "time column
shall be comparable");
+long timestamp = IngestionUtils.extractTimeValue((Comparable) timeValue);
+RecordLocation location = new RecordLocation(_segmentName, docId,
timestamp);
+// check local primary key index first
+if (_primaryKeyIndex.containsKey(primaryKey)) {
+ RecordLocation prevLocation = _primaryKeyIndex.get(primaryKey);
+ if (location.getTimestamp() >= prevLocation.getTimestamp()) {
+_primaryKeyIndex.put(primaryKey, location);
+// update validDocIndex
+_validDocIndex.remove(prevLocation.getDocId());
+_validDocIndex.checkAndAdd(location.getDocId());
+LOGGER.debug(String
+.format("upsert: replace old doc id %d with %d for key: %s, hash:
%d", prevLocation.getDocId(),
+location.getDocId(), primaryKey, primaryKey.hashCode()));
+ } else {
+LOGGER.debug(
+String.format("upsert: ignore a late-arrived record: %s, hash:
%d", primaryKey, primaryKey.hashCode()));
+ }
+} else if (_partitionUpsertMetadataManager.containsKey(primaryKey)) {
+ RecordLocation prevLocation =
_partitionUpsertMetadataManager.getRecordLocation(primaryKey);
+ if (location.getTimestamp() >= prevLocation.getTimestamp()) {
+_partitionUpsertMetadataManager.removeRecordLocation(primaryKey);
+_primaryKeyIndex.put(primaryKey, location);
+
+// update validDocIndex
+
_partitionUpsertMetadataManager.getValidDocIndex(prevLocation.getSegmentName())
+.remove(prevLocation.getDocId());
+_validDocIndex.checkAndAdd(location.getDocId());
+ }
+} else {
+ _primaryKeyIndex.put(primaryKey, location);
+ _validDocIndex.checkAndAdd(location.getDocId());
+}
Review comment:
+1
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java
##
@@ -266,14 +292,52 @@ public void addSegment(String segmentName, TableConfig
tableConfig, IndexLoading
manager =
new LLRealtimeSegmentDataManager(realtimeSegmentZKMetadata,
tableConfig, this, _indexDir.getAbsolutePath(),
indexLoadingConfig, schema, llcSegmentName,
_partitionIdToSemaphoreMap.get(streamPartitionId),
-_serverMetrics);
+_serverMetrics, _tableUpsertMetadataManager);
}
_logger.info("Initialize RealtimeSegmentDataManager - " + segmentName);
_segmentDataManagerMap.put(segmentName, manager);
_serverMetrics.addValueToTableGauge(_tableNameWithType,
ServerGauge.SEGMENT_COUNT, 1L);
}
}
+ private boolean isUpsertEnabled() {
+return _upsertMode == UpsertConfig.Mode.FULL || _upsertMode ==
UpsertConfig.Mode.PARTIAL;
+ }
+
+ @Override
+ public void
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6124: RealtimeToOfflineSegments task generator
mcvsubbu commented on a change in pull request #6124:
URL: https://github.com/apache/incubator-pinot/pull/6124#discussion_r503594418
##
File path:
pinot-minion/src/main/java/org/apache/pinot/minion/executor/PinotTaskExecutor.java
##
@@ -26,6 +26,12 @@
*/
public interface PinotTaskExecutor {
+ /**
+ * Pre processing operations to be done at the beginning of task execution
+ */
+ default void preProcess(PinotTaskConfig pinotTaskConfig) {
Review comment:
Why not make this a boolean return, so that if the pre-process fails we
do not go further in the task execution? Not sure what helix does when we throw
exceptions from the task executor -- probably retries the task, but that may
not be good for us.
##
File path:
pinot-minion/src/main/java/org/apache/pinot/minion/executor/BaseSingleSegmentConversionExecutor.java
##
@@ -135,6 +137,8 @@ public SegmentConversionResult executeTask(PinotTaskConfig
pinotTaskConfig)
convertedSegmentTarFile);
LOGGER.info("Done executing {} on table: {}, segment: {}", taskType,
tableNameWithType, segmentName);
+ postProcess(pinotTaskConfig);
Review comment:
please move the log line to be below postProcess, since it says "Done"
It may be useful to have a log line after each phase
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6134: Adding a flag in PinotServiceManager to allow health check includes all components health check
codecov-io edited a comment on pull request #6134: URL: https://github.com/apache/incubator-pinot/pull/6134#issuecomment-707351920 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6134?src=pr=h1) Report > Merging [#6134](https://codecov.io/gh/apache/incubator-pinot/pull/6134?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) will **increase** coverage by `6.35%`. > The diff coverage is `59.69%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/6134?src=pr=tree) ```diff @@Coverage Diff @@ ## master#6134 +/- ## == + Coverage 66.44% 72.80% +6.35% == Files1075 1223 +148 Lines 5477357744+2971 Branches 8168 8522 +354 == + Hits3639642038+5642 + Misses 1570012941-2759 - Partials 2677 2765 +88 ``` | Flag | Coverage Δ | | |---|---|---| | #integration | `45.21% <48.75%> (?)` | | | #unittests | `64.04% <38.00%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6134?src=pr=tree) | Coverage Δ | | |---|---|---| | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | | | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: | | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `53.33% <0.00%> (-3.81%)` | :arrow_down: | | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `44.44% <0.00%> (-4.40%)` | :arrow_down: | | [.../org/apache/pinot/client/ResultTableResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFRhYmxlUmVzdWx0U2V0LmphdmE=) | `24.00% <0.00%> (-10.29%)` | :arrow_down: | | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `78.57% <ø> (+5.40%)` | :arrow_up: | | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: | | [...pinot/common/function/AggregationFunctionType.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vQWdncmVnYXRpb25GdW5jdGlvblR5cGUuamF2YQ==) | `98.27% <ø> (-1.73%)` | :arrow_down: | | [.../pinot/common/function/DateTimePatternHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRGF0ZVRpbWVQYXR0ZXJuSGFuZGxlci5qYXZh) | `83.33% <ø> (ø)` | | | [...ot/common/function/FunctionDefinitionRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRnVuY3Rpb25EZWZpbml0aW9uUmVnaXN0cnkuamF2YQ==) | `88.88% <ø> (+44.44%)` | :arrow_up: | | ... and [973 more](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree-more) | | -- [Continue to review full report at
[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6112: add max length support in schema builder
codecov-io edited a comment on pull request #6112: URL: https://github.com/apache/incubator-pinot/pull/6112#issuecomment-704563263 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=h1) Report > Merging [#6112](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) will **increase** coverage by `6.35%`. > The diff coverage is `59.53%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=tree) ```diff @@Coverage Diff @@ ## master#6112 +/- ## == + Coverage 66.44% 72.79% +6.35% == Files1075 1223 +148 Lines 5477357752+2979 Branches 8168 8524 +356 == + Hits3639642043+5647 + Misses 1570012944-2756 - Partials 2677 2765 +88 ``` | Flag | Coverage Δ | | |---|---|---| | #integration | `45.21% <48.55%> (?)` | | | #unittests | `64.02% <37.57%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=tree) | Coverage Δ | | |---|---|---| | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | | | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: | | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `53.33% <0.00%> (-3.81%)` | :arrow_down: | | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `44.44% <0.00%> (-4.40%)` | :arrow_down: | | [.../org/apache/pinot/client/ResultTableResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFRhYmxlUmVzdWx0U2V0LmphdmE=) | `24.00% <0.00%> (-10.29%)` | :arrow_down: | | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `78.57% <ø> (+5.40%)` | :arrow_up: | | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: | | [...pinot/common/function/AggregationFunctionType.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vQWdncmVnYXRpb25GdW5jdGlvblR5cGUuamF2YQ==) | `98.27% <ø> (-1.73%)` | :arrow_down: | | [.../pinot/common/function/DateTimePatternHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRGF0ZVRpbWVQYXR0ZXJuSGFuZGxlci5qYXZh) | `83.33% <ø> (ø)` | | | [...ot/common/function/FunctionDefinitionRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRnVuY3Rpb25EZWZpbml0aW9uUmVnaXN0cnkuamF2YQ==) | `88.88% <ø> (+44.44%)` | :arrow_up: | | ... and [974 more](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree-more) | | -- [Continue to review full report at
[GitHub] [incubator-pinot] vincentchenjl opened a new pull request #6135: [TE] Add thirdeye-dashboard dep to thirdeye-dist
vincentchenjl opened a new pull request #6135: URL: https://github.com/apache/incubator-pinot/pull/6135 This PR is to fix the failure when maven runs `maven-assembly-plugin` before the `maven-shade-plugin`, which causes that maven cannot find `thirdeye-dashboard-1.0.0-SNAPSHOT.jar` for building the .zip file. Error message is shown as following: `thirdeye-dist: Failed to create assembly: Error adding file to archive: /path/to/incubator-pinot/thirdeye/thirdeye-dist/thirdeye/thirdeye-dist/../thirdeye-dashboard/target/thirdeye-dashboard-1.0.0-SNAPSHOT.jar isn't a file` Steps to reproduce the issue: 1. Run `mvn clean`. Make sure that all the `target/` directories are deleted. 2. Run `mvn install -DskipTests` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] deemoliu commented on a change in pull request #6112: add max length support in schema builder
deemoliu commented on a change in pull request #6112:
URL: https://github.com/apache/incubator-pinot/pull/6112#discussion_r503541176
##
File path:
pinot-common/src/test/java/org/apache/pinot/common/data/FieldSpecTest.java
##
@@ -80,6 +80,18 @@ public void testFieldSpec() {
Assert.assertEquals(fieldSpec1.hashCode(), fieldSpec2.hashCode());
Assert.assertEquals(fieldSpec1.getDefaultNullValue(), "false");
+// Single-value boolean type dimension field with max length and default
null value.
+fieldSpec1 = new DimensionFieldSpec();
+fieldSpec1.setName("svDimension");
+fieldSpec1.setDataType(BOOLEAN);
+fieldSpec1.setDefaultNullValue(false);
+fieldSpec1.setMaxLength(2);
Review comment:
@chenboat @Jackie-Jiang thanks for the code review. I added precondition
in Schema.java for dimension columns. Regarding metrics column, does METRIC
column support String column?
I think it should be supported according to
https://docs.pinot.apache.org/configuration-reference/schema#metricfieldspecs
However in the schema.java it weren't been supported.
https://github.com/apache/incubator-pinot/blob/master/pinot-spi/src/main/java/org/apache/pinot/spi/data/Schema.java#L447
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on issue #6121: Reduce ingestion latency during segment sealing
mcvsubbu commented on issue #6121: URL: https://github.com/apache/incubator-pinot/issues/6121#issuecomment-707370550 Follow up discussion in slack This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6112: add max length support in schema builder
codecov-io edited a comment on pull request #6112: URL: https://github.com/apache/incubator-pinot/pull/6112#issuecomment-704563263 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=h1) Report > Merging [#6112](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) will **decrease** coverage by `21.01%`. > The diff coverage is `48.55%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#6112 +/- ## === - Coverage 66.44% 45.43% -21.02% === Files1075 1223 +148 Lines 5477357757 +2984 Branches 8168 8525 +357 === - Hits3639626244-10152 - Misses 1570029368+13668 + Partials 2677 2145 -532 ``` | Flag | Coverage Δ | | |---|---|---| | #integration | `45.43% <48.55%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=tree) | Coverage Δ | | |---|---|---| | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | | | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: | | [...org/apache/pinot/broker/queryquota/HitCounter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9IaXRDb3VudGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: | | [...che/pinot/broker/queryquota/MaxHitRateTracker.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9NYXhIaXRSYXRlVHJhY2tlci5qYXZh) | `0.00% <0.00%> (ø)` | | | [...ache/pinot/broker/queryquota/QueryQuotaEntity.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9RdWVyeVF1b3RhRW50aXR5LmphdmE=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: | | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `26.66% <0.00%> (-30.48%)` | :arrow_down: | | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `22.22% <0.00%> (-26.62%)` | :arrow_down: | | [.../org/apache/pinot/client/ResultTableResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFRhYmxlUmVzdWx0U2V0LmphdmE=) | `24.00% <0.00%> (-10.29%)` | :arrow_down: | | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `64.28% <ø> (-8.89%)` | :arrow_down: | | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: | | ... and [1204 more](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=continue). > **Legend** -
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6124: RealtimeToOfflineSegments task generator
mcvsubbu commented on a change in pull request #6124:
URL: https://github.com/apache/incubator-pinot/pull/6124#discussion_r503580109
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/generator/RealtimeToOfflineSegmentsTaskGenerator.java
##
@@ -0,0 +1,238 @@
+/**
+ * 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.minion.generator;
+
+import com.google.common.base.Preconditions;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.helix.task.TaskState;
+import org.apache.pinot.common.metadata.segment.LLCRealtimeSegmentZKMetadata;
+import org.apache.pinot.common.metadata.segment.RealtimeSegmentZKMetadata;
+import org.apache.pinot.common.minion.RealtimeToOfflineSegmentsTaskMetadata;
+import org.apache.pinot.common.utils.CommonConstants.Segment;
+import org.apache.pinot.controller.helix.core.minion.ClusterInfoProvider;
+import org.apache.pinot.core.common.MinionConstants;
+import
org.apache.pinot.core.common.MinionConstants.RealtimeToOfflineSegmentsTask;
+import org.apache.pinot.core.minion.PinotTaskConfig;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableTaskConfig;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.utils.TimeUtils;
+import org.apache.pinot.spi.utils.builder.TableNameBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * A {@link PinotTaskGenerator} implementation for generating tasks of type
{@link RealtimeToOfflineSegmentsTask}
+ *
+ * These will be generated only for REALTIME tables.
+ * At any given time, only 1 task of this type should be generated for a table.
+ *
+ * Steps:
+ * - The watermarkMillis is read from the {@link
RealtimeToOfflineSegmentsTaskMetadata} ZNode found at
MINION_TASK_METADATA/realtimeToOfflineSegmentsTask/tableNameWithType
+ * In case of cold-start, no ZNode will exist.
+ * A new ZNode will be created, with watermarkMillis as the smallest time
found in the COMPLETED segments (or using start time config)
+ * - The execution window for the task is calculated as, windowStartMillis =
waterMarkMillis, windowEndMillis = windowStartMillis + bucketTimeMillis,
+ * where bucketTime can be provided in the taskConfigs (default 1d)
+ * - If the execution window is not older than bufferTimeMillis, no task will
be generated,
+ * where bufferTime can be provided in the taskConfigs (default 2d)
+ * - Segment metadata is scanned for all COMPLETED segments, to pick those
containing data in window [windowStartMillis, windowEndMillis)
+ * - A PinotTaskConfig is created, with segment information, execution
window, and any config specific to the task
+ */
+public class RealtimeToOfflineSegmentsTaskGenerator implements
PinotTaskGenerator {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(RealtimeToOfflineSegmentsTaskGenerator.class);
+
+ private static final String DEFAULT_BUCKET_PERIOD = "1d";
+ private static final String DEFAULT_BUFFER_PERIOD = "2d";
+
+ private final ClusterInfoProvider _clusterInfoProvider;
+
+ public RealtimeToOfflineSegmentsTaskGenerator(ClusterInfoProvider
clusterInfoProvider) {
+_clusterInfoProvider = clusterInfoProvider;
+ }
+
+ @Override
+ public String getTaskType() {
+return RealtimeToOfflineSegmentsTask.TASK_TYPE;
+ }
+
+ @Override
+ public List generateTasks(List tableConfigs) {
Review comment:
It will improve readability if you break this method down to call
helpers.
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/generator/RealtimeToOfflineSegmentsTaskGenerator.java
##
@@ -0,0 +1,238 @@
+/**
+ * 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
+ *
[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6112: add max length support in schema builder
codecov-io edited a comment on pull request #6112: URL: https://github.com/apache/incubator-pinot/pull/6112#issuecomment-704563263 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=h1) Report > Merging [#6112](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) will **decrease** coverage by `21.23%`. > The diff coverage is `48.55%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#6112 +/- ## === - Coverage 66.44% 45.21% -21.24% === Files1075 1223 +148 Lines 5477357752 +2979 Branches 8168 8524 +356 === - Hits3639626111-10285 - Misses 1570029501+13801 + Partials 2677 2140 -537 ``` | Flag | Coverage Δ | | |---|---|---| | #integration | `45.21% <48.55%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=tree) | Coverage Δ | | |---|---|---| | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | | | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: | | [...org/apache/pinot/broker/queryquota/HitCounter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9IaXRDb3VudGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: | | [...che/pinot/broker/queryquota/MaxHitRateTracker.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9NYXhIaXRSYXRlVHJhY2tlci5qYXZh) | `0.00% <0.00%> (ø)` | | | [...ache/pinot/broker/queryquota/QueryQuotaEntity.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9RdWVyeVF1b3RhRW50aXR5LmphdmE=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: | | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `26.66% <0.00%> (-30.48%)` | :arrow_down: | | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `22.22% <0.00%> (-26.62%)` | :arrow_down: | | [.../org/apache/pinot/client/ResultTableResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFRhYmxlUmVzdWx0U2V0LmphdmE=) | `24.00% <0.00%> (-10.29%)` | :arrow_down: | | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `64.28% <ø> (-8.89%)` | :arrow_down: | | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: | | ... and [1204 more](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=continue). > **Legend** -
[incubator-pinot] branch throw-exception-when-column-mismatch updated (16167e9 -> bb210aa)
This is an automated email from the ASF dual-hosted git repository. jlli pushed a change to branch throw-exception-when-column-mismatch in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard 16167e9 Add warn level message and emit metric if this behavior detected discard c1f022b Address PR comments discard e0987e5 Add option to fail query when column mismatches add 1c4fc13 Added additional time format option for druid (#6060) add 2379791 Move scalar function from commons to plugins package (#6064) add 26f6019 [TE] ThirdEye Coordinator skeleton stub (#6065) add f556c59 [TE] Fixed tests on ThirdEye dashboard for MacOS (#6062) add f196dfc Add Hadoop related dependencies in pinot-tool module (#6070) add e892cb2 Enhance DistinctCountThetaSketchAggregationFunction (#6004) add 40cb64d Add list of allowed tables for emitting table level metrics (#6037) add 8d79e0a Add toString() to AggregationFunctionColumnPair (#6077) add 009ab53 Add FilterOptimizer which supports optimizing both PQL and SQL query filter (#6056) add e5cdb1e Allow configurable controller vip (#6071) add 4f2e767 Adding push job type of segment metadata only mode (#5967) add f4d9630 Refresh the routing when realtime segment is committed (#6078) add 7a40f11 [TE] show alert health on the alerts page (#6072) add 6c4f3c7 [TE] frontend - harleyjj/yaml - show spinner in buttons when submitting yaml config for creating or editing detection or subscription group (#6051) add a0dcc66 Minion taskExecutor for RealtimeToOfflineSegments task (#6050) add e303938 [TE] upgrade dropwizard-swagger dependency (#6076) add 14332cd Fix StarTreeClusterIntegrationTest by not removing the segments (#6087) add 8083b61 Adding array transform functions: array_average, array_max, array_min, array_sum (#6084) add deb3891 Add support for Decimal with Precision Sum aggregation (#6053) add a8a6ab2 Enhance AggregationFunctionColumnPair to accept underscore in function name (#6079) add 9929dad Adding more table config validation (#6073) add 267abef [TE] Refactor. ThirdEye Principal should be immutable. (#6085) add 1126cac [TE] move dashboard resources for refactoring (#6058) add e4d7a10 Add a property to set the s3 endpoint (#6104) add 93238c9 add upsert related configs (#6096) add 2afea5c Fix missing segment count reporting for realtime llc segment (#6103) add 11ff74a Making pushType non-mandatory (#6107) add 24147dd [Issue 6068] Fixing the calls to Helix to throw exception if zk conne… (#6069) add be99d78 Add the primary key reading from the GenericRow (#6102) add b658925 Allow modifying/removing existing star-trees during segment reload (#6100) add 1b9dfb5 [TE] Datalayer refactor. Reorganizing Guice Module inside DaoProviderUtil (#6108) add 8782e47 Fixed indexing link (#6110) add ac3f2af FIX Homepage changelog link (#6116) add 02dd3e2 Adding Tenants, Instances, Tables, Segments count tiles and their respective pages (#6117) add 81028ce Enhance star-tree to skip matching-all predicate on non-star-tree dimension (#6109) add 4a60e9b Create swagger dir and put api dir to swagger dir in pinot-controller resource (#6122) add 0e1d458 Implement off-heap bloom filter reader (#6118) add fd78e6a Make transform functions support underscore in the query functions (#6125) add 9e757ef [TE] add owners field into the subscription validation (#6128) add 6275818 Fix superset docker image build script (#5965) add 5667515 Add option to fail query when column mismatches add 0b89a06 Address PR comments add 25eccab Add warn level message and emit metric if this behavior detected add bb210aa Address PR comments This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (16167e9) \ N -- N -- N refs/heads/throw-exception-when-column-mismatch (bb210aa) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new revisions were added by this update. Summary of changes: README.md |2 +- docker/images/pinot-superset/.dockerignore |1 - docker/images/pinot-superset/Dockerfile| 120 +- docker/images/pinot-superset/README.md | 10 +- docker/images/pinot-superset/bin/superset-init | 13 -
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #6066: Detect the behavior when column name mismatches in the query
jackjlli commented on a change in pull request #6066:
URL: https://github.com/apache/incubator-pinot/pull/6066#discussion_r503610423
##
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
##
@@ -769,94 +786,133 @@ private void updateColumnNames(BrokerRequest
brokerRequest) {
for (int i = 0; i < selectionColumns.size(); i++) {
String expression = selectionColumns.get(i);
if (!expression.equals("*")) {
- selectionColumns.set(i, fixColumnName(rawTableName, expression,
columnNameMap));
+ selectionColumns.set(i, fixColumnName(rawTableName, expression,
columnNameMap, query));
}
}
}
if (brokerRequest.isSetOrderBy()) {
List orderBy = brokerRequest.getOrderBy();
for (SelectionSort selectionSort : orderBy) {
String expression = selectionSort.getColumn();
-selectionSort.setColumn(fixColumnName(rawTableName, expression,
columnNameMap));
+selectionSort.setColumn(fixColumnName(rawTableName, expression,
columnNameMap, query));
}
}
PinotQuery pinotQuery = brokerRequest.getPinotQuery();
if (pinotQuery != null) {
for (Expression expression : pinotQuery.getSelectList()) {
-fixColumnName(rawTableName, expression, columnNameMap);
+fixColumnName(rawTableName, expression, columnNameMap, query);
}
Expression filterExpression = pinotQuery.getFilterExpression();
if (filterExpression != null) {
-fixColumnName(rawTableName, filterExpression, columnNameMap);
+fixColumnName(rawTableName, filterExpression, columnNameMap, query);
}
List groupByList = pinotQuery.getGroupByList();
if (groupByList != null) {
for (Expression expression : groupByList) {
- fixColumnName(rawTableName, expression, columnNameMap);
+ fixColumnName(rawTableName, expression, columnNameMap, query);
}
}
List orderByList = pinotQuery.getOrderByList();
if (orderByList != null) {
for (Expression expression : orderByList) {
- fixColumnName(rawTableName, expression, columnNameMap);
+ fixColumnName(rawTableName, expression, columnNameMap, query);
}
}
Expression havingExpression = pinotQuery.getHavingExpression();
if (havingExpression != null) {
-fixColumnName(rawTableName, havingExpression, columnNameMap);
+fixColumnName(rawTableName, havingExpression, columnNameMap, query);
}
}
}
- private String fixColumnName(String rawTableName, String expression,
@Nullable Map columnNameMap) {
+ private String fixColumnName(String rawTableName, String expression,
@Nullable Map columnNameMap,
+ String query) {
TransformExpressionTree expressionTree =
TransformExpressionTree.compileToExpressionTree(expression);
-fixColumnName(rawTableName, expressionTree, columnNameMap);
+fixColumnName(rawTableName, expressionTree, columnNameMap, query);
return expressionTree.toString();
}
private void fixColumnName(String rawTableName, TransformExpressionTree
expression,
- @Nullable Map columnNameMap) {
+ @Nullable Map columnNameMap, String query) {
TransformExpressionTree.ExpressionType expressionType =
expression.getExpressionType();
if (expressionType == TransformExpressionTree.ExpressionType.IDENTIFIER) {
- expression.setValue(getActualColumnName(rawTableName,
expression.getValue(), columnNameMap));
+ expression.setValue(
+ getActualColumnName(rawTableName, expression.getValue(),
columnNameMap, query));
} else if (expressionType ==
TransformExpressionTree.ExpressionType.FUNCTION) {
for (TransformExpressionTree child : expression.getChildren()) {
-fixColumnName(rawTableName, child, columnNameMap);
+fixColumnName(rawTableName, child, columnNameMap, query);
}
}
}
- private void fixColumnName(String rawTableName, Expression expression,
@Nullable Map columnNameMap) {
+ private void fixColumnName(String rawTableName, Expression expression,
@Nullable Map columnNameMap,
+ String query) {
ExpressionType expressionType = expression.getType();
if (expressionType == ExpressionType.IDENTIFIER) {
- Identifier identifier = expression.getIdentifier();
- identifier.setName(getActualColumnName(rawTableName,
identifier.getName(), columnNameMap));
+ if (!isStarIdentifier(expression)) {
+Identifier identifier = expression.getIdentifier();
+identifier.setName(getActualColumnName(rawTableName,
identifier.getName(), columnNameMap,
+query));
+ }
} else if (expressionType == ExpressionType.FUNCTION) {
- for (Expression operand : expression.getFunctionCall().getOperands()) {
-fixColumnName(rawTableName, operand, columnNameMap);
+ String operator =
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #6066: Detect the behavior when column name mismatches in the query
jackjlli commented on a change in pull request #6066:
URL: https://github.com/apache/incubator-pinot/pull/6066#discussion_r503610443
##
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
##
@@ -769,94 +786,133 @@ private void updateColumnNames(BrokerRequest
brokerRequest) {
for (int i = 0; i < selectionColumns.size(); i++) {
String expression = selectionColumns.get(i);
if (!expression.equals("*")) {
- selectionColumns.set(i, fixColumnName(rawTableName, expression,
columnNameMap));
+ selectionColumns.set(i, fixColumnName(rawTableName, expression,
columnNameMap, query));
}
}
}
if (brokerRequest.isSetOrderBy()) {
List orderBy = brokerRequest.getOrderBy();
for (SelectionSort selectionSort : orderBy) {
String expression = selectionSort.getColumn();
-selectionSort.setColumn(fixColumnName(rawTableName, expression,
columnNameMap));
+selectionSort.setColumn(fixColumnName(rawTableName, expression,
columnNameMap, query));
}
}
PinotQuery pinotQuery = brokerRequest.getPinotQuery();
if (pinotQuery != null) {
for (Expression expression : pinotQuery.getSelectList()) {
-fixColumnName(rawTableName, expression, columnNameMap);
+fixColumnName(rawTableName, expression, columnNameMap, query);
}
Expression filterExpression = pinotQuery.getFilterExpression();
if (filterExpression != null) {
-fixColumnName(rawTableName, filterExpression, columnNameMap);
+fixColumnName(rawTableName, filterExpression, columnNameMap, query);
}
List groupByList = pinotQuery.getGroupByList();
if (groupByList != null) {
for (Expression expression : groupByList) {
- fixColumnName(rawTableName, expression, columnNameMap);
+ fixColumnName(rawTableName, expression, columnNameMap, query);
}
}
List orderByList = pinotQuery.getOrderByList();
if (orderByList != null) {
for (Expression expression : orderByList) {
- fixColumnName(rawTableName, expression, columnNameMap);
+ fixColumnName(rawTableName, expression, columnNameMap, query);
}
}
Expression havingExpression = pinotQuery.getHavingExpression();
if (havingExpression != null) {
-fixColumnName(rawTableName, havingExpression, columnNameMap);
+fixColumnName(rawTableName, havingExpression, columnNameMap, query);
}
}
}
- private String fixColumnName(String rawTableName, String expression,
@Nullable Map columnNameMap) {
+ private String fixColumnName(String rawTableName, String expression,
@Nullable Map columnNameMap,
+ String query) {
TransformExpressionTree expressionTree =
TransformExpressionTree.compileToExpressionTree(expression);
-fixColumnName(rawTableName, expressionTree, columnNameMap);
+fixColumnName(rawTableName, expressionTree, columnNameMap, query);
return expressionTree.toString();
}
private void fixColumnName(String rawTableName, TransformExpressionTree
expression,
- @Nullable Map columnNameMap) {
+ @Nullable Map columnNameMap, String query) {
TransformExpressionTree.ExpressionType expressionType =
expression.getExpressionType();
if (expressionType == TransformExpressionTree.ExpressionType.IDENTIFIER) {
- expression.setValue(getActualColumnName(rawTableName,
expression.getValue(), columnNameMap));
+ expression.setValue(
+ getActualColumnName(rawTableName, expression.getValue(),
columnNameMap, query));
} else if (expressionType ==
TransformExpressionTree.ExpressionType.FUNCTION) {
for (TransformExpressionTree child : expression.getChildren()) {
-fixColumnName(rawTableName, child, columnNameMap);
+fixColumnName(rawTableName, child, columnNameMap, query);
}
}
}
- private void fixColumnName(String rawTableName, Expression expression,
@Nullable Map columnNameMap) {
+ private void fixColumnName(String rawTableName, Expression expression,
@Nullable Map columnNameMap,
+ String query) {
ExpressionType expressionType = expression.getType();
if (expressionType == ExpressionType.IDENTIFIER) {
- Identifier identifier = expression.getIdentifier();
- identifier.setName(getActualColumnName(rawTableName,
identifier.getName(), columnNameMap));
+ if (!isStarIdentifier(expression)) {
+Identifier identifier = expression.getIdentifier();
+identifier.setName(getActualColumnName(rawTableName,
identifier.getName(), columnNameMap,
+query));
+ }
} else if (expressionType == ExpressionType.FUNCTION) {
- for (Expression operand : expression.getFunctionCall().getOperands()) {
-fixColumnName(rawTableName, operand, columnNameMap);
+ String operator =
[incubator-pinot] branch throw-exception-when-column-mismatch updated (bb210aa -> 7c6b34e)
This is an automated email from the ASF dual-hosted git repository. jlli pushed a change to branch throw-exception-when-column-mismatch in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard bb210aa Address PR comments add 7c6b34e Address PR comments This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (bb210aa) \ N -- N -- N refs/heads/throw-exception-when-column-mismatch (7c6b34e) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new revisions were added by this update. Summary of changes: .../requesthandler/BaseBrokerRequestHandler.java | 50 ++ 1 file changed, 23 insertions(+), 27 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io commented on pull request #6134: Adding a flag in PinotServiceManager to allow health check includes all components health check
codecov-io commented on pull request #6134: URL: https://github.com/apache/incubator-pinot/pull/6134#issuecomment-707351920 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6134?src=pr=h1) Report > Merging [#6134](https://codecov.io/gh/apache/incubator-pinot/pull/6134?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) will **increase** coverage by `6.34%`. > The diff coverage is `59.69%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/6134?src=pr=tree) ```diff @@Coverage Diff @@ ## master#6134 +/- ## == + Coverage 66.44% 72.79% +6.34% == Files1075 1223 +148 Lines 5477357744+2971 Branches 8168 8522 +354 == + Hits3639642033+5637 + Misses 1570012945-2755 - Partials 2677 2766 +89 ``` | Flag | Coverage Δ | | |---|---|---| | #integration | `45.21% <48.75%> (?)` | | | #unittests | `64.02% <38.00%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6134?src=pr=tree) | Coverage Δ | | |---|---|---| | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | | | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: | | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `53.33% <0.00%> (-3.81%)` | :arrow_down: | | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `44.44% <0.00%> (-4.40%)` | :arrow_down: | | [.../org/apache/pinot/client/ResultTableResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFRhYmxlUmVzdWx0U2V0LmphdmE=) | `24.00% <0.00%> (-10.29%)` | :arrow_down: | | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `78.57% <ø> (+5.40%)` | :arrow_up: | | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: | | [...pinot/common/function/AggregationFunctionType.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vQWdncmVnYXRpb25GdW5jdGlvblR5cGUuamF2YQ==) | `98.27% <ø> (-1.73%)` | :arrow_down: | | [.../pinot/common/function/DateTimePatternHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRGF0ZVRpbWVQYXR0ZXJuSGFuZGxlci5qYXZh) | `83.33% <ø> (ø)` | | | [...ot/common/function/FunctionDefinitionRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRnVuY3Rpb25EZWZpbml0aW9uUmVnaXN0cnkuamF2YQ==) | `88.88% <ø> (+44.44%)` | :arrow_up: | | ... and [971 more](https://codecov.io/gh/apache/incubator-pinot/pull/6134/diff?src=pr=tree-more) | | -- [Continue to review full report at
[GitHub] [incubator-pinot] deemoliu commented on a change in pull request #6112: add max length support in schema builder
deemoliu commented on a change in pull request #6112: URL: https://github.com/apache/incubator-pinot/pull/6112#discussion_r503562057 ## File path: pinot-spi/src/main/java/org/apache/pinot/spi/data/Schema.java ## @@ -499,6 +499,15 @@ public SchemaBuilder addSingleValueDimension(String dimensionName, DataType data return this; } +/** + * Add single value dimensionFieldSpec with maxLength and a defaultNullValue + */ +public SchemaBuilder addSingleValueDimension(String dimensionName, DataType dataType, int maxLength, Review comment: gotcha, added precondition check. thanks for review. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6112: add max length support in schema builder
codecov-io edited a comment on pull request #6112: URL: https://github.com/apache/incubator-pinot/pull/6112#issuecomment-704563263 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=h1) Report > Merging [#6112](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) will **increase** coverage by `6.42%`. > The diff coverage is `59.53%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=tree) ```diff @@Coverage Diff @@ ## master#6112 +/- ## == + Coverage 66.44% 72.86% +6.42% == Files1075 1223 +148 Lines 5477357757+2984 Branches 8168 8525 +357 == + Hits3639642087+5691 + Misses 1570012906-2794 - Partials 2677 2764 +87 ``` | Flag | Coverage Δ | | |---|---|---| | #integration | `45.43% <48.55%> (?)` | | | #unittests | `64.01% <37.57%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6112?src=pr=tree) | Coverage Δ | | |---|---|---| | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | | | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: | | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `53.33% <0.00%> (-3.81%)` | :arrow_down: | | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `44.44% <0.00%> (-4.40%)` | :arrow_down: | | [.../org/apache/pinot/client/ResultTableResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFRhYmxlUmVzdWx0U2V0LmphdmE=) | `24.00% <0.00%> (-10.29%)` | :arrow_down: | | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `78.57% <ø> (+5.40%)` | :arrow_up: | | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: | | [...pinot/common/function/AggregationFunctionType.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vQWdncmVnYXRpb25GdW5jdGlvblR5cGUuamF2YQ==) | `98.27% <ø> (-1.73%)` | :arrow_down: | | [.../pinot/common/function/DateTimePatternHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRGF0ZVRpbWVQYXR0ZXJuSGFuZGxlci5qYXZh) | `83.33% <ø> (ø)` | | | [...ot/common/function/FunctionDefinitionRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRnVuY3Rpb25EZWZpbml0aW9uUmVnaXN0cnkuamF2YQ==) | `88.88% <ø> (+44.44%)` | :arrow_up: | | ... and [977 more](https://codecov.io/gh/apache/incubator-pinot/pull/6112/diff?src=pr=tree-more) | | -- [Continue to review full report at
[GitHub] [incubator-pinot] jihaozh merged pull request #6097: Fix Thirdeye Maven publish
jihaozh merged pull request #6097: URL: https://github.com/apache/incubator-pinot/pull/6097 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated (6275818 -> 86ce7c6)
This is an automated email from the ASF dual-hosted git repository. jihao pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. from 6275818 Fix superset docker image build script (#5965) add 86ce7c6 [TE] Fix Thirdeye Maven publish (#6097) No new revisions were added by this update. Summary of changes: .travis/.travis_te_nightly_build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6044: Support for multi-threaded Group By reducer for SQL.
mayankshriv commented on a change in pull request #6044:
URL: https://github.com/apache/incubator-pinot/pull/6044#discussion_r503637782
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/BrokerReduceService.java
##
@@ -47,8 +57,32 @@
@ThreadSafe
public class BrokerReduceService {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(BrokerReduceService.class);
+
+ // brw -> Shorthand for broker reduce worker threads.
+ private static final String REDUCE_THREAD_NAME_FORMAT = "brw-%d";
+ protected static final int QUERY_RUNNER_THREAD_PRIORITY = 7;
Review comment:
This is the same as the server side code, will add comments.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6044: Support for multi-threaded Group By reducer for SQL.
mayankshriv commented on a change in pull request #6044:
URL: https://github.com/apache/incubator-pinot/pull/6044#discussion_r503637680
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/BrokerReduceService.java
##
@@ -47,8 +57,32 @@
@ThreadSafe
public class BrokerReduceService {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(BrokerReduceService.class);
+
+ // brw -> Shorthand for broker reduce worker threads.
+ private static final String REDUCE_THREAD_NAME_FORMAT = "brw-%d";
+ protected static final int QUERY_RUNNER_THREAD_PRIORITY = 7;
+
+ private final ListeningExecutorService _reduceExecutorService;
+ private final int _maxReduceThreadsPerQuery;
+
+ public BrokerReduceService(PinotConfiguration config) {
+_maxReduceThreadsPerQuery =
config.getProperty(CommonConstants.Broker.CONFIG_OF_MAX_REDUCE_THREADS_PER_QUERY,
+CommonConstants.Broker.MAX_REDUCE_THREADS_PER_QUERY);
+LOGGER.info("Initializing BrokerReduceService with {} reduce threads.",
_maxReduceThreadsPerQuery);
Review comment:
Initially, I had Guava's `MoreExecutor.directorExecutor()` that uses the
current thread to run the task, in case of single thread. I decided to just
keep it simple and have the exact same code in case of single vs multi-thread
(with exception of index table). We can revisit that if needed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] yupeng9 commented on a change in pull request #6113: Adding the upsert support to real-time ingestion and query
yupeng9 commented on a change in pull request #6113:
URL: https://github.com/apache/incubator-pinot/pull/6113#discussion_r503668532
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java
##
@@ -132,6 +146,19 @@ protected void doInit() {
String consumerDirPath = getConsumerDir();
File consumerDir = new File(consumerDirPath);
+TableConfig tableConfig =
ZKMetadataProvider.getTableConfig(_propertyStore, _tableNameWithType);
Review comment:
hmm, I did not see how to access it but from `_propertyStore `. Can you
be more specific?
Also, I am not sure the upsert config can change dynamically: partial vs
full seems a disruptive change.
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java
##
@@ -266,14 +292,52 @@ public void addSegment(String segmentName, TableConfig
tableConfig, IndexLoading
manager =
new LLRealtimeSegmentDataManager(realtimeSegmentZKMetadata,
tableConfig, this, _indexDir.getAbsolutePath(),
indexLoadingConfig, schema, llcSegmentName,
_partitionIdToSemaphoreMap.get(streamPartitionId),
-_serverMetrics);
+_serverMetrics, _tableUpsertMetadataManager);
}
_logger.info("Initialize RealtimeSegmentDataManager - " + segmentName);
_segmentDataManagerMap.put(segmentName, manager);
_serverMetrics.addValueToTableGauge(_tableNameWithType,
ServerGauge.SEGMENT_COUNT, 1L);
}
}
+ private boolean isUpsertEnabled() {
+return _upsertMode == UpsertConfig.Mode.FULL || _upsertMode ==
UpsertConfig.Mode.PARTIAL;
+ }
+
+ @Override
+ public void addSegment(ImmutableSegment immutableSegment) {
+if (isUpsertEnabled()) {
+ handleUpsert(immutableSegment);
+}
+super.addSegment(immutableSegment);
+ }
+
+ private void handleUpsert(ImmutableSegment immutableSegment) {
Review comment:
Do you suggest creating a `RealTimeUpsertTableDataManager` in
`TableDataManagerProvider.getTableDataManager`? It's an option to me, though I
feel uspert shall be better built as a first-class citizen in realtime table.
@Jackie-Jiang what do you think?
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/indexsegment/mutable/MutableSegmentImpl.java
##
@@ -453,6 +465,49 @@ public boolean index(GenericRow row, @Nullable RowMetadata
rowMetadata) {
return canTakeMore;
}
+ private boolean isUpsertEnabled() {
+return _upsertMode != null && _upsertMode != UpsertConfig.Mode.NONE;
+ }
+
+ private void handleUpsert(GenericRow row, int docId) {
+// below are upsert operations
+PrimaryKey primaryKey = row.getPrimaryKey(_schema.getPrimaryKeyColumns());
+Object timeValue = row.getValue(_timeColumnName);
+Preconditions.checkArgument(timeValue instanceof Comparable, "time column
shall be comparable");
+long timestamp = IngestionUtils.extractTimeValue((Comparable) timeValue);
+RecordLocation location = new RecordLocation(_segmentName, docId,
timestamp);
+// check local primary key index first
+if (_primaryKeyIndex.containsKey(primaryKey)) {
+ RecordLocation prevLocation = _primaryKeyIndex.get(primaryKey);
+ if (location.getTimestamp() >= prevLocation.getTimestamp()) {
+_primaryKeyIndex.put(primaryKey, location);
+// update validDocIndex
+_validDocIndex.remove(prevLocation.getDocId());
+_validDocIndex.checkAndAdd(location.getDocId());
+LOGGER.debug(String
+.format("upsert: replace old doc id %d with %d for key: %s, hash:
%d", prevLocation.getDocId(),
+location.getDocId(), primaryKey, primaryKey.hashCode()));
+ } else {
+LOGGER.debug(
+String.format("upsert: ignore a late-arrived record: %s, hash:
%d", primaryKey, primaryKey.hashCode()));
+ }
+} else if (_partitionUpsertMetadataManager.containsKey(primaryKey)) {
+ RecordLocation prevLocation =
_partitionUpsertMetadataManager.getRecordLocation(primaryKey);
+ if (location.getTimestamp() >= prevLocation.getTimestamp()) {
+_partitionUpsertMetadataManager.removeRecordLocation(primaryKey);
+_primaryKeyIndex.put(primaryKey, location);
+
+// update validDocIndex
+
_partitionUpsertMetadataManager.getValidDocIndex(prevLocation.getSegmentName())
+.remove(prevLocation.getDocId());
+_validDocIndex.checkAndAdd(location.getDocId());
+ }
+} else {
+ _primaryKeyIndex.put(primaryKey, location);
+ _validDocIndex.checkAndAdd(location.getDocId());
+}
Review comment:
okay, let me refactor this a bit.
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java
##
@@ -1165,6 +1172,14 @@ public
[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6044: Support for multi-threaded Group By reducer for SQL.
codecov-io edited a comment on pull request #6044: URL: https://github.com/apache/incubator-pinot/pull/6044#issuecomment-707475996 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6044?src=pr=h1) Report > Merging [#6044](https://codecov.io/gh/apache/incubator-pinot/pull/6044?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) will **decrease** coverage by `2.42%`. > The diff coverage is `38.09%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/6044?src=pr=tree) ```diff @@Coverage Diff @@ ## master#6044 +/- ## == - Coverage 66.44% 64.02% -2.43% == Files1075 1224 +149 Lines 5477357805+3032 Branches 8168 8528 +360 == + Hits3639637010 +614 - Misses 1570018103+2403 - Partials 2677 2692 +15 ``` | Flag | Coverage Δ | | |---|---|---| | #unittests | `64.02% <38.09%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6044?src=pr=tree) | Coverage Δ | | |---|---|---| | [...e/pinot/broker/api/resources/PinotBrokerDebug.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdEJyb2tlckRlYnVnLmphdmE=) | `0.00% <0.00%> (-79.32%)` | :arrow_down: | | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `71.42% <ø> (-28.58%)` | :arrow_down: | | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `33.96% <0.00%> (-32.71%)` | :arrow_down: | | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `53.33% <0.00%> (-3.81%)` | :arrow_down: | | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `35.55% <0.00%> (-13.29%)` | :arrow_down: | | [...inot/client/JsonAsyncHttpPinotClientTransport.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0pzb25Bc3luY0h0dHBQaW5vdENsaWVudFRyYW5zcG9ydC5qYXZh) | `10.90% <0.00%> (-51.10%)` | :arrow_down: | | [.../org/apache/pinot/client/ResultTableResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFRhYmxlUmVzdWx0U2V0LmphdmE=) | `0.00% <0.00%> (-34.29%)` | :arrow_down: | | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `73.80% <ø> (+0.63%)` | :arrow_up: | | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: | | [...pinot/common/function/AggregationFunctionType.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vQWdncmVnYXRpb25GdW5jdGlvblR5cGUuamF2YQ==) | `98.27% <ø> (-1.73%)` | :arrow_down: | | ... and [1047 more](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree-more) | | -- [Continue to review full report at
[incubator-pinot] branch throw-exception-when-column-mismatch updated (7c6b34e -> 1c01c8b)
This is an automated email from the ASF dual-hosted git repository. jlli pushed a change to branch throw-exception-when-column-mismatch in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard 7c6b34e Address PR comments add 1c01c8b Address PR comments This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (7c6b34e) \ N -- N -- N refs/heads/throw-exception-when-column-mismatch (1c01c8b) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new revisions were added by this update. Summary of changes: .../requesthandler/BaseBrokerRequestHandler.java | 20 +--- .../tests/OfflineClusterIntegrationTest.java | 6 +- 2 files changed, 6 insertions(+), 20 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io commented on pull request #6066: Detect the behavior when column name mismatches in the query
codecov-io commented on pull request #6066: URL: https://github.com/apache/incubator-pinot/pull/6066#issuecomment-707443440 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6066?src=pr=h1) Report > Merging [#6066](https://codecov.io/gh/apache/incubator-pinot/pull/6066?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) will **decrease** coverage by `20.98%`. > The diff coverage is `50.18%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/6066?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#6066 +/- ## === - Coverage 66.44% 45.46% -20.99% === Files1075 1223 +148 Lines 5477357770 +2997 Branches 8168 8528 +360 === - Hits3639626265-10131 - Misses 1570029356+13656 + Partials 2677 2149 -528 ``` | Flag | Coverage Δ | | |---|---|---| | #integration | `45.46% <50.18%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6066?src=pr=tree) | Coverage Δ | | |---|---|---| | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | | | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: | | [...org/apache/pinot/broker/queryquota/HitCounter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9IaXRDb3VudGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: | | [...che/pinot/broker/queryquota/MaxHitRateTracker.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9NYXhIaXRSYXRlVHJhY2tlci5qYXZh) | `0.00% <0.00%> (ø)` | | | [...ache/pinot/broker/queryquota/QueryQuotaEntity.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9RdWVyeVF1b3RhRW50aXR5LmphdmE=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: | | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `26.66% <0.00%> (-30.48%)` | :arrow_down: | | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `22.22% <0.00%> (-26.62%)` | :arrow_down: | | [.../org/apache/pinot/client/ResultTableResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFRhYmxlUmVzdWx0U2V0LmphdmE=) | `24.00% <0.00%> (-10.29%)` | :arrow_down: | | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `64.28% <ø> (-8.89%)` | :arrow_down: | | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: | | ... and [1206 more](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6066?src=pr=continue). > **Legend** - [Click
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6044: Support for multi-threaded Group By reducer for SQL.
mayankshriv commented on a change in pull request #6044: URL: https://github.com/apache/incubator-pinot/pull/6044#discussion_r503636090 ## File path: pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java ## @@ -166,6 +166,11 @@ public static final double DEFAULT_BROKER_MIN_RESOURCE_PERCENT_FOR_START = 100.0; public static final String CONFIG_OF_ENABLE_QUERY_LIMIT_OVERRIDE = "pinot.broker.enable.query.limit.override"; +// Config for number of threads to use for Broker reduce-phase. +public static final String CONFIG_OF_MAX_REDUCE_THREADS_PER_QUERY = "pinot.broker.max.reduce.threads"; Review comment: I debated about it, and felt that sub-setting becomes interesting (e.g. what does 'per' mean). But seems like there's other configs that also follow this, so will change. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. 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
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6044: Support for multi-threaded Group By reducer for SQL.
mayankshriv commented on a change in pull request #6044: URL: https://github.com/apache/incubator-pinot/pull/6044#discussion_r503635525 ## File path: pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java ## @@ -102,7 +102,7 @@ protected final AtomicLong _requestIdGenerator = new AtomicLong(); protected final BrokerRequestOptimizer _brokerRequestOptimizer = new BrokerRequestOptimizer(); - protected final BrokerReduceService _brokerReduceService = new BrokerReduceService(); + protected BrokerReduceService _brokerReduceService; Review comment: Yeah, not sure what happened there. Probably a side effect of trying out some intermediate code. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6044: Support for multi-threaded Group By reducer for SQL.
codecov-io edited a comment on pull request #6044: URL: https://github.com/apache/incubator-pinot/pull/6044#issuecomment-707475996 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6044?src=pr=h1) Report > Merging [#6044](https://codecov.io/gh/apache/incubator-pinot/pull/6044?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) will **increase** coverage by `6.41%`. > The diff coverage is `60.00%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/6044?src=pr=tree) ```diff @@Coverage Diff @@ ## master#6044 +/- ## == + Coverage 66.44% 72.86% +6.41% == Files1075 1224 +149 Lines 5477357805+3032 Branches 8168 8528 +360 == + Hits3639642118+5722 + Misses 1570012928-2772 - Partials 2677 2759 +82 ``` | Flag | Coverage Δ | | |---|---|---| | #integration | `45.44% <49.14%> (?)` | | | #unittests | `64.02% <38.09%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6044?src=pr=tree) | Coverage Δ | | |---|---|---| | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | | | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: | | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `53.33% <0.00%> (-3.81%)` | :arrow_down: | | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `44.44% <0.00%> (-4.40%)` | :arrow_down: | | [.../org/apache/pinot/client/ResultTableResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFRhYmxlUmVzdWx0U2V0LmphdmE=) | `24.00% <0.00%> (-10.29%)` | :arrow_down: | | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `78.57% <ø> (+5.40%)` | :arrow_up: | | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: | | [...pinot/common/function/AggregationFunctionType.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vQWdncmVnYXRpb25GdW5jdGlvblR5cGUuamF2YQ==) | `98.27% <ø> (-1.73%)` | :arrow_down: | | [.../pinot/common/function/DateTimePatternHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRGF0ZVRpbWVQYXR0ZXJuSGFuZGxlci5qYXZh) | `83.33% <ø> (ø)` | | | [...ot/common/function/FunctionDefinitionRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRnVuY3Rpb25EZWZpbml0aW9uUmVnaXN0cnkuamF2YQ==) | `88.88% <ø> (+44.44%)` | :arrow_up: | | ... and [972 more](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree-more) | | -- [Continue to review full report at
[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6066: Detect the behavior when column name mismatches in the query
codecov-io edited a comment on pull request #6066: URL: https://github.com/apache/incubator-pinot/pull/6066#issuecomment-707443440 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6066?src=pr=h1) Report > Merging [#6066](https://codecov.io/gh/apache/incubator-pinot/pull/6066?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) will **increase** coverage by `6.39%`. > The diff coverage is `60.50%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/6066?src=pr=tree) ```diff @@Coverage Diff @@ ## master#6066 +/- ## == + Coverage 66.44% 72.84% +6.39% == Files1075 1223 +148 Lines 5477357770+2997 Branches 8168 8528 +360 == + Hits3639642081+5685 + Misses 1570012927-2773 - Partials 2677 2762 +85 ``` | Flag | Coverage Δ | | |---|---|---| | #integration | `45.46% <50.18%> (?)` | | | #unittests | `63.97% <36.05%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6066?src=pr=tree) | Coverage Δ | | |---|---|---| | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | | | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: | | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `53.33% <0.00%> (-3.81%)` | :arrow_down: | | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `44.44% <0.00%> (-4.40%)` | :arrow_down: | | [.../org/apache/pinot/client/ResultTableResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFRhYmxlUmVzdWx0U2V0LmphdmE=) | `24.00% <0.00%> (-10.29%)` | :arrow_down: | | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `78.57% <ø> (+5.40%)` | :arrow_up: | | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: | | [...pinot/common/function/AggregationFunctionType.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vQWdncmVnYXRpb25GdW5jdGlvblR5cGUuamF2YQ==) | `98.27% <ø> (-1.73%)` | :arrow_down: | | [.../pinot/common/function/DateTimePatternHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRGF0ZVRpbWVQYXR0ZXJuSGFuZGxlci5qYXZh) | `83.33% <ø> (ø)` | | | [...ot/common/function/FunctionDefinitionRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRnVuY3Rpb25EZWZpbml0aW9uUmVnaXN0cnkuamF2YQ==) | `88.88% <ø> (+44.44%)` | :arrow_up: | | ... and [974 more](https://codecov.io/gh/apache/incubator-pinot/pull/6066/diff?src=pr=tree-more) | | -- [Continue to review full report at
[GitHub] [incubator-pinot] npawar commented on a change in pull request #6094: Implement the segment merge task generator
npawar commented on a change in pull request #6094:
URL: https://github.com/apache/incubator-pinot/pull/6094#discussion_r503600877
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/mergestrategy/MergeStrategy.java
##
@@ -0,0 +1,28 @@
+/**
+ * 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.minion.mergestrategy;
+
+import java.util.List;
+import org.apache.pinot.common.metadata.segment.SegmentZKMetadata;
+
+
+public interface MergeStrategy {
Review comment:
Some javadoc for MergeStrategy and MergeStrategyFactory would be nice
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/mergestrategy/MergeStrategy.java
##
@@ -0,0 +1,28 @@
+/**
+ * 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.minion.mergestrategy;
+
+import java.util.List;
+import org.apache.pinot.common.metadata.segment.SegmentZKMetadata;
+
+
+public interface MergeStrategy {
+ List> computeSegmentsToMerge(List
segmentsToMerge,
Review comment:
should segmentsToMerge be "allSegments" ? It's confusing that we're
sending segmentToMerge to a method called computeSegmentsToMerge.
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/generator/SegmentMergeRollupTaskGenerator.java
##
@@ -0,0 +1,200 @@
+/**
+ * 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.minion.generator;
+
+import com.google.common.base.Preconditions;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.pinot.common.lineage.LineageEntry;
+import org.apache.pinot.common.lineage.LineageEntryState;
+import org.apache.pinot.common.lineage.SegmentLineage;
+import org.apache.pinot.common.metadata.segment.OfflineSegmentZKMetadata;
+import org.apache.pinot.common.metadata.segment.SegmentZKMetadata;
+import org.apache.pinot.controller.helix.core.minion.ClusterInfoProvider;
+import
org.apache.pinot.controller.helix.core.minion.mergestrategy.MergeStrategyFactory;
+import org.apache.pinot.core.common.MinionConstants;
+import org.apache.pinot.core.minion.PinotTaskConfig;
+import org.apache.pinot.core.segment.processing.collector.CollectorFactory;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableTaskConfig;
+import org.apache.pinot.spi.config.table.TableType;
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6044: Support for multi-threaded Group By reducer for SQL.
mayankshriv commented on a change in pull request #6044:
URL: https://github.com/apache/incubator-pinot/pull/6044#discussion_r503640236
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java
##
@@ -231,58 +239,130 @@ private DataSchema
getPrePostAggregationDataSchema(DataSchema dataSchema) {
return new DataSchema(columnNames, columnDataTypes);
}
- private IndexedTable getIndexedTable(DataSchema dataSchema,
Collection dataTables) {
+ private IndexedTable getIndexedTable(DataSchema dataSchema,
Collection dataTablesToReduce,
+ DataTableReducerContext reducerContext) {
+long start = System.currentTimeMillis();
+int numDataTables = dataTablesToReduce.size();
+
+// Get the number of threads to use for reducing.
+int numReduceThreadsToUse = getNumReduceThreadsToUse(numDataTables,
reducerContext.getMaxReduceThreadsPerQuery());
+
+// In case of single reduce thread, fall back to SimpleIndexedTable to
avoid redundant locking/unlocking calls.
int capacity = GroupByUtils.getTableCapacity(_queryContext);
-IndexedTable indexedTable = new SimpleIndexedTable(dataSchema,
_queryContext, capacity);
+IndexedTable indexedTable =
+(numReduceThreadsToUse > 1) ? new ConcurrentIndexedTable(dataSchema,
_queryContext, capacity)
+: new SimpleIndexedTable(dataSchema, _queryContext, capacity);
+
+Future[] futures = new Future[numDataTables];
+CountDownLatch countDownLatch = new CountDownLatch(numDataTables);
+
+// Create groups of data tables that each thread can process concurrently.
+// Given that numReduceThreads is <= numDataTables, each group will have
at least one data table.
+ArrayList dataTables = new ArrayList<>(dataTablesToReduce);
+List> reduceGroups = new
ArrayList<>(numReduceThreadsToUse);
+
+for (int i = 0; i < numReduceThreadsToUse; i++) {
+ reduceGroups.add(new ArrayList<>());
+}
+for (int i = 0; i < numDataTables; i++) {
+ reduceGroups.get(i % numReduceThreadsToUse).add(dataTables.get(i));
+}
+
+int cnt = 0;
ColumnDataType[] columnDataTypes = dataSchema.getColumnDataTypes();
-for (DataTable dataTable : dataTables) {
- int numRows = dataTable.getNumberOfRows();
- for (int rowId = 0; rowId < numRows; rowId++) {
-Object[] values = new Object[_numColumns];
-for (int colId = 0; colId < _numColumns; colId++) {
- switch (columnDataTypes[colId]) {
-case INT:
- values[colId] = dataTable.getInt(rowId, colId);
- break;
-case LONG:
- values[colId] = dataTable.getLong(rowId, colId);
- break;
-case FLOAT:
- values[colId] = dataTable.getFloat(rowId, colId);
- break;
-case DOUBLE:
- values[colId] = dataTable.getDouble(rowId, colId);
- break;
-case STRING:
- values[colId] = dataTable.getString(rowId, colId);
- break;
-case BYTES:
- values[colId] = dataTable.getBytes(rowId, colId);
- break;
-case OBJECT:
- values[colId] = dataTable.getObject(rowId, colId);
- break;
-// Add other aggregation intermediate result / group-by column
type supports here
-default:
- throw new IllegalStateException();
+for (List reduceGroup : reduceGroups) {
+ futures[cnt++] = reducerContext.getExecutorService().submit(new
TraceRunnable() {
+@Override
+public void runJob() {
+ for (DataTable dataTable : reduceGroup) {
+int numRows = dataTable.getNumberOfRows();
+
+try {
+ for (int rowId = 0; rowId < numRows; rowId++) {
+Object[] values = new Object[_numColumns];
+for (int colId = 0; colId < _numColumns; colId++) {
+ switch (columnDataTypes[colId]) {
+case INT:
+ values[colId] = dataTable.getInt(rowId, colId);
+ break;
+case LONG:
+ values[colId] = dataTable.getLong(rowId, colId);
+ break;
+case FLOAT:
+ values[colId] = dataTable.getFloat(rowId, colId);
+ break;
+case DOUBLE:
+ values[colId] = dataTable.getDouble(rowId, colId);
+ break;
+case STRING:
+ values[colId] = dataTable.getString(rowId, colId);
+ break;
+case BYTES:
+ values[colId] = dataTable.getBytes(rowId, colId);
+ break;
+case OBJECT:
+ values[colId] = dataTable.getObject(rowId, colId);
+ break;
+
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6044: Support for multi-threaded Group By reducer for SQL.
mayankshriv commented on a change in pull request #6044:
URL: https://github.com/apache/incubator-pinot/pull/6044#discussion_r503640614
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java
##
@@ -231,58 +239,130 @@ private DataSchema
getPrePostAggregationDataSchema(DataSchema dataSchema) {
return new DataSchema(columnNames, columnDataTypes);
}
- private IndexedTable getIndexedTable(DataSchema dataSchema,
Collection dataTables) {
+ private IndexedTable getIndexedTable(DataSchema dataSchema,
Collection dataTablesToReduce,
+ DataTableReducerContext reducerContext) {
+long start = System.currentTimeMillis();
+int numDataTables = dataTablesToReduce.size();
+
+// Get the number of threads to use for reducing.
+int numReduceThreadsToUse = getNumReduceThreadsToUse(numDataTables,
reducerContext.getMaxReduceThreadsPerQuery());
+
+// In case of single reduce thread, fall back to SimpleIndexedTable to
avoid redundant locking/unlocking calls.
int capacity = GroupByUtils.getTableCapacity(_queryContext);
-IndexedTable indexedTable = new SimpleIndexedTable(dataSchema,
_queryContext, capacity);
+IndexedTable indexedTable =
+(numReduceThreadsToUse > 1) ? new ConcurrentIndexedTable(dataSchema,
_queryContext, capacity)
+: new SimpleIndexedTable(dataSchema, _queryContext, capacity);
+
+Future[] futures = new Future[numDataTables];
Review comment:
I really don't want to have two separate implementations (one for single
thread and one for multi-thread). One way to do that is to use DirectExecutor
that uses current thread to launch the job. Does that work?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io commented on pull request #6044: Support for multi-threaded Group By reducer for SQL.
codecov-io commented on pull request #6044: URL: https://github.com/apache/incubator-pinot/pull/6044#issuecomment-707475996 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6044?src=pr=h1) Report > Merging [#6044](https://codecov.io/gh/apache/incubator-pinot/pull/6044?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) will **decrease** coverage by `21.12%`. > The diff coverage is `49.14%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/6044?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#6044 +/- ## === - Coverage 66.44% 45.32% -21.13% === Files1075 1224 +149 Lines 5477357796 +3023 Branches 8168 8528 +360 === - Hits3639626197-10199 - Misses 1570029458+13758 + Partials 2677 2141 -536 ``` | Flag | Coverage Δ | | |---|---|---| | #integration | `45.32% <49.14%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6044?src=pr=tree) | Coverage Δ | | |---|---|---| | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | | | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: | | [...org/apache/pinot/broker/queryquota/HitCounter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9IaXRDb3VudGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: | | [...che/pinot/broker/queryquota/MaxHitRateTracker.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9NYXhIaXRSYXRlVHJhY2tlci5qYXZh) | `0.00% <0.00%> (ø)` | | | [...ache/pinot/broker/queryquota/QueryQuotaEntity.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9RdWVyeVF1b3RhRW50aXR5LmphdmE=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: | | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `26.66% <0.00%> (-30.48%)` | :arrow_down: | | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `22.22% <0.00%> (-26.62%)` | :arrow_down: | | [.../org/apache/pinot/client/ResultTableResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFRhYmxlUmVzdWx0U2V0LmphdmE=) | `24.00% <0.00%> (-10.29%)` | :arrow_down: | | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `64.28% <ø> (-8.89%)` | :arrow_down: | | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: | | ... and [1205 more](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6044?src=pr=continue). > **Legend** - [Click
[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6044: Support for multi-threaded Group By reducer for SQL.
codecov-io edited a comment on pull request #6044: URL: https://github.com/apache/incubator-pinot/pull/6044#issuecomment-707475996 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6044?src=pr=h1) Report > Merging [#6044](https://codecov.io/gh/apache/incubator-pinot/pull/6044?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) will **increase** coverage by `6.40%`. > The diff coverage is `60.00%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/6044?src=pr=tree) ```diff @@Coverage Diff @@ ## master#6044 +/- ## == + Coverage 66.44% 72.85% +6.40% == Files1075 1224 +149 Lines 5477357796+3023 Branches 8168 8528 +360 == + Hits3639642106+5710 + Misses 1570012927-2773 - Partials 2677 2763 +86 ``` | Flag | Coverage Δ | | |---|---|---| | #integration | `45.32% <49.14%> (?)` | | | #unittests | `64.02% <38.09%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6044?src=pr=tree) | Coverage Δ | | |---|---|---| | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | | | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: | | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `53.33% <0.00%> (-3.81%)` | :arrow_down: | | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `44.44% <0.00%> (-4.40%)` | :arrow_down: | | [.../org/apache/pinot/client/ResultTableResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFRhYmxlUmVzdWx0U2V0LmphdmE=) | `24.00% <0.00%> (-10.29%)` | :arrow_down: | | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `78.57% <ø> (+5.40%)` | :arrow_up: | | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: | | [...pinot/common/function/AggregationFunctionType.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vQWdncmVnYXRpb25GdW5jdGlvblR5cGUuamF2YQ==) | `98.27% <ø> (-1.73%)` | :arrow_down: | | [.../pinot/common/function/DateTimePatternHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRGF0ZVRpbWVQYXR0ZXJuSGFuZGxlci5qYXZh) | `83.33% <ø> (ø)` | | | [...ot/common/function/FunctionDefinitionRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRnVuY3Rpb25EZWZpbml0aW9uUmVnaXN0cnkuamF2YQ==) | `88.88% <ø> (+44.44%)` | :arrow_up: | | ... and [976 more](https://codecov.io/gh/apache/incubator-pinot/pull/6044/diff?src=pr=tree-more) | | -- [Continue to review full report at
