You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/07/12 23:30:41 UTC
[GitHub] [incubator-pinot] Jackie-Jiang opened a new pull request #7158: Extract time handling for SegmentProcessorFramework
Jackie-Jiang opened a new pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158
## Description
Currently within `SegmentProcessorFramework`, time handling is modeled as separate general filter/transform/partition function within the processor config, which is quite hard to configure and much less efficient. Also, record filter/transform should be configured in the table config and handled within the `RecordTransformer` instead of in the framework, or it can cause inconsistency.
This PR extracts the time handling logic for SegmentProcessorFramework and support:
- Time filtering: filter time within a time range
- Time rounding: Round time according to the round time bucket
- Time partitioning: Partition time according to the partition time bucket
Currently only `EPOCH` mode is supported for time handling. Will add `DATE_TIME` mode in the future to support round/partition time on calendar date boundary with timezone support.
## Release Notes
In `RealtimeToOfflineSegmentsTask` config:
- `timeColumnTransformFunction` is removed (backward-incompatible, but rollup is not supported anyway)
- Deprecate `collectorType` and replace it with `mergeType`
- Add `roundBucketTimePeriod` and `partitionBucketTimePeriod` to config the time bucket for round and partition
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on a change in pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158#discussion_r669999594
##########
File path: pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/realtime_to_offline_segments/RealtimeToOfflineSegmentsTaskGenerator.java
##########
@@ -207,29 +207,35 @@ public String getTaskType() {
configs.put(MinionConstants.DOWNLOAD_URL_KEY, StringUtils.join(downloadURLs, MinionConstants.URL_SEPARATOR));
configs.put(MinionConstants.UPLOAD_URL_KEY, _clusterInfoAccessor.getVipUrl() + "/segments");
- // Execution window
+ // Segment processor configs
configs.put(RealtimeToOfflineSegmentsTask.WINDOW_START_MS_KEY, String.valueOf(windowStartMs));
configs.put(RealtimeToOfflineSegmentsTask.WINDOW_END_MS_KEY, String.valueOf(windowEndMs));
-
- // Segment processor configs
- String timeColumnTransformationConfig =
- taskConfigs.get(RealtimeToOfflineSegmentsTask.TIME_COLUMN_TRANSFORM_FUNCTION_KEY);
- if (timeColumnTransformationConfig != null) {
- configs.put(RealtimeToOfflineSegmentsTask.TIME_COLUMN_TRANSFORM_FUNCTION_KEY, timeColumnTransformationConfig);
+ String roundBucketTimePeriod = taskConfigs.get(RealtimeToOfflineSegmentsTask.ROUND_BUCKET_TIME_PERIOD_KEY);
+ if (roundBucketTimePeriod != null) {
+ configs.put(RealtimeToOfflineSegmentsTask.ROUND_BUCKET_TIME_PERIOD_KEY, roundBucketTimePeriod);
Review comment:
can you give an example of when a user might have to use the PARTITION_BUCKET_TIME_PERIOD? and how will it look in the config?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158#issuecomment-878690453
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7158](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b0909ae) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/34e82571ede9c36ca6fa20d9ec2cdc3d80af7ee5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (34e8257) will **decrease** coverage by `8.26%`.
> The diff coverage is `89.72%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7158 +/- ##
============================================
- Coverage 73.55% 65.29% -8.27%
Complexity 92 92
============================================
Files 1500 1505 +5
Lines 73758 73794 +36
Branches 10637 10642 +5
============================================
- Hits 54256 48186 -6070
- Misses 15965 22203 +6238
+ Partials 3537 3405 -132
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `?` | |
| unittests | `65.29% <89.72%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../org/apache/pinot/core/common/MinionConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vTWluaW9uQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (ø)` | |
| [...processing/partitioner/ColumnValuePartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvQ29sdW1uVmFsdWVQYXJ0aXRpb25lci5qYXZh) | `100.00% <ø> (ø)` | |
| [...egment/processing/partitioner/NoOpPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvTm9PcFBhcnRpdGlvbmVyLmphdmE=) | `100.00% <ø> (ø)` | |
| [...ent/processing/partitioner/PartitionerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvUGFydGl0aW9uZXJGYWN0b3J5LmphdmE=) | `96.00% <ø> (ø)` | |
| [.../processing/partitioner/RoundRobinPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvUm91bmRSb2JpblBhcnRpdGlvbmVyLmphdmE=) | `100.00% <ø> (ø)` | |
| [...processing/partitioner/TableConfigPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvVGFibGVDb25maWdQYXJ0aXRpb25lci5qYXZh) | `100.00% <ø> (ø)` | |
| [...sing/partitioner/TransformFunctionPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvVHJhbnNmb3JtRnVuY3Rpb25QYXJ0aXRpb25lci5qYXZh) | `100.00% <ø> (ø)` | |
| [...ent/processing/timehandler/TimeHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvdGltZWhhbmRsZXIvVGltZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `73.33% <73.33%> (ø)` | |
| [...gments/RealtimeToOfflineSegmentsTaskGenerator.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvcmVhbHRpbWVfdG9fb2ZmbGluZV9zZWdtZW50cy9SZWFsdGltZVRvT2ZmbGluZVNlZ21lbnRzVGFza0dlbmVyYXRvci5qYXZh) | `93.18% <73.33%> (-1.48%)` | :arrow_down: |
| [.../core/segment/processing/mapper/SegmentMapper.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvbWFwcGVyL1NlZ21lbnRNYXBwZXIuamF2YQ==) | `88.13% <78.57%> (+1.46%)` | :arrow_up: |
| ... and [375 more](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [34e8257...b0909ae](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] snleee commented on a change in pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
snleee commented on a change in pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158#discussion_r669342288
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/segment/processing/timehandler/TimeHandlerConfig.java
##########
@@ -0,0 +1,100 @@
+/**
+ * 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.core.segment.processing.timehandler;
+
+/**
+ * Config for TimeHandler.
+ */
+public class TimeHandlerConfig {
+ private final TimeHandler.Type _type;
+ private final long _startTimeMs;
+ private final long _endTimeMs;
+
+ // For EPOCH handler
+ private final long _roundBucketMs;
Review comment:
Can you add a bit more documentation around `roundBucketMs` and `partitionBucketMs`?
Should we add `period` to the variable name? It seems that those 2 values are bucket size (time period).
##########
File path: pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/realtime_to_offline_segments/RealtimeToOfflineSegmentsTaskExecutor.java
##########
@@ -132,67 +130,66 @@ public void preProcess(PinotTaskConfig pinotTaskConfig) {
String offlineTableName = TableNameBuilder.OFFLINE.tableNameWithType(rawTableName);
TableConfig tableConfig = getTableConfig(offlineTableName);
Schema schema = getSchema(offlineTableName);
- Set<String> schemaColumns = schema.getPhysicalColumnNames();
String timeColumn = tableConfig.getValidationConfig().getTimeColumnName();
DateTimeFieldSpec dateTimeFieldSpec = schema.getSpecForTimeColumn(timeColumn);
Preconditions
.checkState(dateTimeFieldSpec != null, "No valid spec found for time column: %s in schema for table: %s",
timeColumn, offlineTableName);
+ SegmentProcessorConfig.Builder segmentProcessorConfigBuilder =
+ new SegmentProcessorConfig.Builder().setTableConfig(tableConfig).setSchema(schema);
- long windowStartMs = Long.parseLong(configs.get(MinionConstants.RealtimeToOfflineSegmentsTask.WINDOW_START_MS_KEY));
- long windowEndMs = Long.parseLong(configs.get(MinionConstants.RealtimeToOfflineSegmentsTask.WINDOW_END_MS_KEY));
+ long windowStartMs = Long.parseLong(configs.get(RealtimeToOfflineSegmentsTask.WINDOW_START_MS_KEY));
+ long windowEndMs = Long.parseLong(configs.get(RealtimeToOfflineSegmentsTask.WINDOW_END_MS_KEY));
_nextWatermark = windowEndMs;
- String timeColumnTransformFunction =
Review comment:
Removing `TIME_COLUMN_TRANSFORM_FUNCTION` means that RealtimeToOffline task is not backward compatible after this change.
Two options:
1. Add deprecated annotation & keep the backward compatibility instead of removing this
2. Remove this and include it in the release note
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/segment/processing/timehandler/TimeHandler.java
##########
@@ -0,0 +1,45 @@
+/**
+ * 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.core.segment.processing.timehandler;
+
+import javax.annotation.Nullable;
+import org.apache.pinot.spi.data.readers.GenericRow;
+
+
+/**
+ * Handler for time values within the records:
+ * - Filter records based on the start/end time range
+ * - Round time values (for ROLLUP and DEDUP mode)
Review comment:
Can you briefly explain why `DEDUP` needs rounding?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158#discussion_r669928656
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/segment/processing/timehandler/TimeHandler.java
##########
@@ -0,0 +1,45 @@
+/**
+ * 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.core.segment.processing.timehandler;
+
+import javax.annotation.Nullable;
+import org.apache.pinot.spi.data.readers.GenericRow;
+
+
+/**
+ * Handler for time values within the records:
+ * - Filter records based on the start/end time range
+ * - Round time values (for ROLLUP and DEDUP mode)
Review comment:
Time handling can be combined with any merge type per the requirement, and they work independently. I can see potential use cases that requires dedupping the records with same dimensions but different time value (e.g. user always asks for distinct values).
Since TimeHandler is not associated with any merge type, modified the javadoc to not mention the merge type 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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158#issuecomment-878690453
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7158](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c2f26c7) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/de2f0e04dca8130a09ea902787a75997b70cc16d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (de2f0e0) will **increase** coverage by `0.03%`.
> The diff coverage is `89.47%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7158 +/- ##
============================================
+ Coverage 73.49% 73.52% +0.03%
Complexity 92 92
============================================
Files 1500 1505 +5
Lines 73756 73799 +43
Branches 10637 10644 +7
============================================
+ Hits 54207 54261 +54
+ Misses 16012 15998 -14
- Partials 3537 3540 +3
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `41.91% <63.15%> (+<0.01%)` | :arrow_up: |
| unittests | `65.28% <87.50%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../org/apache/pinot/core/common/MinionConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vTWluaW9uQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (ø)` | |
| [...processing/partitioner/ColumnValuePartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvQ29sdW1uVmFsdWVQYXJ0aXRpb25lci5qYXZh) | `100.00% <ø> (ø)` | |
| [...egment/processing/partitioner/NoOpPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvTm9PcFBhcnRpdGlvbmVyLmphdmE=) | `100.00% <ø> (ø)` | |
| [...ent/processing/partitioner/PartitionerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvUGFydGl0aW9uZXJGYWN0b3J5LmphdmE=) | `96.00% <ø> (ø)` | |
| [.../processing/partitioner/RoundRobinPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvUm91bmRSb2JpblBhcnRpdGlvbmVyLmphdmE=) | `100.00% <ø> (ø)` | |
| [...processing/partitioner/TableConfigPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvVGFibGVDb25maWdQYXJ0aXRpb25lci5qYXZh) | `100.00% <ø> (ø)` | |
| [...sing/partitioner/TransformFunctionPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvVHJhbnNmb3JtRnVuY3Rpb25QYXJ0aXRpb25lci5qYXZh) | `100.00% <ø> (ø)` | |
| [...ent/processing/timehandler/TimeHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvdGltZWhhbmRsZXIvVGltZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `73.33% <73.33%> (ø)` | |
| [.../core/segment/processing/mapper/SegmentMapper.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvbWFwcGVyL1NlZ21lbnRNYXBwZXIuamF2YQ==) | `88.13% <78.57%> (+1.46%)` | :arrow_up: |
| [...gments/RealtimeToOfflineSegmentsTaskGenerator.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvcmVhbHRpbWVfdG9fb2ZmbGluZV9zZWdtZW50cy9SZWFsdGltZVRvT2ZmbGluZVNlZ21lbnRzVGFza0dlbmVyYXRvci5qYXZh) | `93.38% <78.94%> (-1.28%)` | :arrow_down: |
| ... and [33 more](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [de2f0e0...c2f26c7](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] snleee commented on a change in pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
snleee commented on a change in pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158#discussion_r669959910
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/segment/processing/timehandler/TimeHandlerConfig.java
##########
@@ -0,0 +1,100 @@
+/**
+ * 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.core.segment.processing.timehandler;
+
+/**
+ * Config for TimeHandler.
+ */
+public class TimeHandlerConfig {
+ private final TimeHandler.Type _type;
+ private final long _startTimeMs;
+ private final long _endTimeMs;
+
+ // For EPOCH handler
+ private final long _roundBucketMs;
Review comment:
Thanks. The comments look good to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-commenter commented on pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158#issuecomment-878690453
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7158](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (72cd1ed) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/de2f0e04dca8130a09ea902787a75997b70cc16d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (de2f0e0) will **decrease** coverage by `8.21%`.
> The diff coverage is `87.50%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7158 +/- ##
============================================
- Coverage 73.49% 65.27% -8.22%
Complexity 92 92
============================================
Files 1500 1505 +5
Lines 73756 73799 +43
Branches 10637 10644 +7
============================================
- Hits 54207 48175 -6032
- Misses 16012 22215 +6203
+ Partials 3537 3409 -128
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `?` | |
| unittests | `65.27% <87.50%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../org/apache/pinot/core/common/MinionConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vTWluaW9uQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (ø)` | |
| [...processing/partitioner/ColumnValuePartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvQ29sdW1uVmFsdWVQYXJ0aXRpb25lci5qYXZh) | `100.00% <ø> (ø)` | |
| [...egment/processing/partitioner/NoOpPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvTm9PcFBhcnRpdGlvbmVyLmphdmE=) | `100.00% <ø> (ø)` | |
| [...ent/processing/partitioner/PartitionerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvUGFydGl0aW9uZXJGYWN0b3J5LmphdmE=) | `96.00% <ø> (ø)` | |
| [.../processing/partitioner/RoundRobinPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvUm91bmRSb2JpblBhcnRpdGlvbmVyLmphdmE=) | `100.00% <ø> (ø)` | |
| [...processing/partitioner/TableConfigPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvVGFibGVDb25maWdQYXJ0aXRpb25lci5qYXZh) | `100.00% <ø> (ø)` | |
| [...sing/partitioner/TransformFunctionPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvVHJhbnNmb3JtRnVuY3Rpb25QYXJ0aXRpb25lci5qYXZh) | `100.00% <ø> (ø)` | |
| [...gments/RealtimeToOfflineSegmentsTaskGenerator.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvcmVhbHRpbWVfdG9fb2ZmbGluZV9zZWdtZW50cy9SZWFsdGltZVRvT2ZmbGluZVNlZ21lbnRzVGFza0dlbmVyYXRvci5qYXZh) | `91.91% <68.42%> (-2.75%)` | :arrow_down: |
| [...ent/processing/timehandler/TimeHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvdGltZWhhbmRsZXIvVGltZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `73.33% <73.33%> (ø)` | |
| [.../core/segment/processing/mapper/SegmentMapper.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvbWFwcGVyL1NlZ21lbnRNYXBwZXIuamF2YQ==) | `88.13% <78.57%> (+1.46%)` | :arrow_up: |
| ... and [378 more](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [de2f0e0...72cd1ed](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158#discussion_r669934253
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/segment/processing/timehandler/TimeHandlerConfig.java
##########
@@ -0,0 +1,100 @@
+/**
+ * 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.core.segment.processing.timehandler;
+
+/**
+ * Config for TimeHandler.
+ */
+public class TimeHandlerConfig {
+ private final TimeHandler.Type _type;
+ private final long _startTimeMs;
+ private final long _endTimeMs;
+
+ // For EPOCH handler
+ private final long _roundBucketMs;
Review comment:
These 2 values are already in millis. The period parsing is done on the caller side.
Added more comments to elaborate the meaning of these 2 configs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158#issuecomment-878690453
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7158](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c2f26c7) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/de2f0e04dca8130a09ea902787a75997b70cc16d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (de2f0e0) will **decrease** coverage by `31.57%`.
> The diff coverage is `63.15%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7158 +/- ##
=============================================
- Coverage 73.49% 41.91% -31.58%
+ Complexity 92 7 -85
=============================================
Files 1500 1505 +5
Lines 73756 73799 +43
Branches 10637 10644 +7
=============================================
- Hits 54207 30936 -23271
- Misses 16012 40239 +24227
+ Partials 3537 2624 -913
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `41.91% <63.15%> (+<0.01%)` | :arrow_up: |
| unittests | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../org/apache/pinot/core/common/MinionConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vTWluaW9uQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (ø)` | |
| [...processing/partitioner/ColumnValuePartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvQ29sdW1uVmFsdWVQYXJ0aXRpb25lci5qYXZh) | `0.00% <ø> (-100.00%)` | :arrow_down: |
| [...egment/processing/partitioner/NoOpPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvTm9PcFBhcnRpdGlvbmVyLmphdmE=) | `0.00% <ø> (-100.00%)` | :arrow_down: |
| [...ent/processing/partitioner/PartitionerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvUGFydGl0aW9uZXJGYWN0b3J5LmphdmE=) | `0.00% <ø> (-96.00%)` | :arrow_down: |
| [.../processing/partitioner/RoundRobinPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvUm91bmRSb2JpblBhcnRpdGlvbmVyLmphdmE=) | `0.00% <ø> (-100.00%)` | :arrow_down: |
| [...processing/partitioner/TableConfigPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvVGFibGVDb25maWdQYXJ0aXRpb25lci5qYXZh) | `0.00% <ø> (-100.00%)` | :arrow_down: |
| [...sing/partitioner/TransformFunctionPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvVHJhbnNmb3JtRnVuY3Rpb25QYXJ0aXRpb25lci5qYXZh) | `0.00% <ø> (-100.00%)` | :arrow_down: |
| [...egment/processing/timehandler/NoOpTimeHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvdGltZWhhbmRsZXIvTm9PcFRpbWVIYW5kbGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...egments/RealtimeToOfflineSegmentsTaskExecutor.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvcmVhbHRpbWVfdG9fb2ZmbGluZV9zZWdtZW50cy9SZWFsdGltZVRvT2ZmbGluZVNlZ21lbnRzVGFza0V4ZWN1dG9yLmphdmE=) | `72.00% <47.36%> (-16.43%)` | :arrow_down: |
| [...gments/RealtimeToOfflineSegmentsTaskGenerator.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvcmVhbHRpbWVfdG9fb2ZmbGluZV9zZWdtZW50cy9SZWFsdGltZVRvT2ZmbGluZVNlZ21lbnRzVGFza0dlbmVyYXRvci5qYXZh) | `71.32% <47.36%> (-23.34%)` | :arrow_down: |
| ... and [961 more](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [de2f0e0...c2f26c7](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158#discussion_r670037480
##########
File path: pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/realtime_to_offline_segments/RealtimeToOfflineSegmentsTaskGenerator.java
##########
@@ -207,29 +207,35 @@ public String getTaskType() {
configs.put(MinionConstants.DOWNLOAD_URL_KEY, StringUtils.join(downloadURLs, MinionConstants.URL_SEPARATOR));
configs.put(MinionConstants.UPLOAD_URL_KEY, _clusterInfoAccessor.getVipUrl() + "/segments");
- // Execution window
+ // Segment processor configs
configs.put(RealtimeToOfflineSegmentsTask.WINDOW_START_MS_KEY, String.valueOf(windowStartMs));
configs.put(RealtimeToOfflineSegmentsTask.WINDOW_END_MS_KEY, String.valueOf(windowEndMs));
-
- // Segment processor configs
- String timeColumnTransformationConfig =
- taskConfigs.get(RealtimeToOfflineSegmentsTask.TIME_COLUMN_TRANSFORM_FUNCTION_KEY);
- if (timeColumnTransformationConfig != null) {
- configs.put(RealtimeToOfflineSegmentsTask.TIME_COLUMN_TRANSFORM_FUNCTION_KEY, timeColumnTransformationConfig);
+ String roundBucketTimePeriod = taskConfigs.get(RealtimeToOfflineSegmentsTask.ROUND_BUCKET_TIME_PERIOD_KEY);
+ if (roundBucketTimePeriod != null) {
+ configs.put(RealtimeToOfflineSegmentsTask.ROUND_BUCKET_TIME_PERIOD_KEY, roundBucketTimePeriod);
Review comment:
Good question. Partitioning on time value is needed for merge/rollup task because it does not filter on the time values. For realtime-to-offline task, it can be useful when the time window covers multiple partition buckets, but we don't generate task this way currently. Removed it for now and we can add it back if necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158#issuecomment-878690453
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7158](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b0909ae) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/34e82571ede9c36ca6fa20d9ec2cdc3d80af7ee5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (34e8257) will **decrease** coverage by `0.08%`.
> The diff coverage is `91.78%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7158 +/- ##
============================================
- Coverage 73.55% 73.47% -0.09%
Complexity 92 92
============================================
Files 1500 1505 +5
Lines 73758 73794 +36
Branches 10637 10642 +5
============================================
- Hits 54256 54222 -34
- Misses 15965 16029 +64
- Partials 3537 3543 +6
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `41.76% <63.69%> (-0.14%)` | :arrow_down: |
| unittests | `65.29% <89.72%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../org/apache/pinot/core/common/MinionConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vTWluaW9uQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (ø)` | |
| [...processing/partitioner/ColumnValuePartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvQ29sdW1uVmFsdWVQYXJ0aXRpb25lci5qYXZh) | `100.00% <ø> (ø)` | |
| [...egment/processing/partitioner/NoOpPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvTm9PcFBhcnRpdGlvbmVyLmphdmE=) | `100.00% <ø> (ø)` | |
| [...ent/processing/partitioner/PartitionerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvUGFydGl0aW9uZXJGYWN0b3J5LmphdmE=) | `96.00% <ø> (ø)` | |
| [.../processing/partitioner/RoundRobinPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvUm91bmRSb2JpblBhcnRpdGlvbmVyLmphdmE=) | `100.00% <ø> (ø)` | |
| [...processing/partitioner/TableConfigPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvVGFibGVDb25maWdQYXJ0aXRpb25lci5qYXZh) | `100.00% <ø> (ø)` | |
| [...sing/partitioner/TransformFunctionPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvVHJhbnNmb3JtRnVuY3Rpb25QYXJ0aXRpb25lci5qYXZh) | `100.00% <ø> (ø)` | |
| [...ent/processing/timehandler/TimeHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvdGltZWhhbmRsZXIvVGltZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `73.33% <73.33%> (ø)` | |
| [.../core/segment/processing/mapper/SegmentMapper.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvbWFwcGVyL1NlZ21lbnRNYXBwZXIuamF2YQ==) | `88.13% <78.57%> (+1.46%)` | :arrow_up: |
| [...gments/RealtimeToOfflineSegmentsTaskGenerator.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvcmVhbHRpbWVfdG9fb2ZmbGluZV9zZWdtZW50cy9SZWFsdGltZVRvT2ZmbGluZVNlZ21lbnRzVGFza0dlbmVyYXRvci5qYXZh) | `94.69% <86.66%> (+0.04%)` | :arrow_up: |
| ... and [52 more](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [34e8257...b0909ae](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jtao15 commented on a change in pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
jtao15 commented on a change in pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158#discussion_r669887600
##########
File path: pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/realtime_to_offline_segments/RealtimeToOfflineSegmentsTaskGenerator.java
##########
@@ -263,13 +269,13 @@ private void getCompletedSegmentsInfo(String realtimeTableName,
if (latestLLCSegmentName == null) {
return llcSegmentName;
} else {
- if (llcSegmentName.getSequenceNumber() > latestLLCSegmentName.getSequenceNumber()) {
- return llcSegmentName;
- } else {
- return latestLLCSegmentName;
- }
- }
- });
+ if (llcSegmentName.getSequenceNumber() > latestLLCSegmentName.getSequenceNumber()) {
Review comment:
Good catch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
npawar commented on pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158#issuecomment-880255215
Please update this as well: https://docs.pinot.apache.org/operators/operating-pinot/pinot-managed-offline-flows
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on a change in pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158#discussion_r669989248
##########
File path: pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/realtime_to_offline_segments/RealtimeToOfflineSegmentsTaskExecutor.java
##########
@@ -26,33 +26,31 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.TimeUnit;
import org.apache.commons.io.FileUtils;
import org.apache.helix.ZNRecord;
import org.apache.pinot.common.metadata.segment.SegmentZKMetadataCustomMapModifier;
import org.apache.pinot.common.minion.RealtimeToOfflineSegmentsTaskMetadata;
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.core.segment.processing.filter.RecordFilterConfig;
-import org.apache.pinot.core.segment.processing.filter.RecordFilterFactory;
import org.apache.pinot.core.segment.processing.framework.MergeType;
import org.apache.pinot.core.segment.processing.framework.SegmentConfig;
import org.apache.pinot.core.segment.processing.framework.SegmentProcessorConfig;
import org.apache.pinot.core.segment.processing.framework.SegmentProcessorFramework;
import org.apache.pinot.core.segment.processing.partitioner.PartitionerConfig;
import org.apache.pinot.core.segment.processing.partitioner.PartitionerFactory;
-import org.apache.pinot.core.segment.processing.transformer.RecordTransformerConfig;
+import org.apache.pinot.core.segment.processing.timehandler.TimeHandler;
+import org.apache.pinot.core.segment.processing.timehandler.TimeHandlerConfig;
import org.apache.pinot.minion.executor.MinionTaskZkMetadataManager;
import org.apache.pinot.plugin.minion.tasks.BaseMultipleSegmentsConversionExecutor;
import org.apache.pinot.plugin.minion.tasks.SegmentConversionResult;
import org.apache.pinot.segment.spi.AggregationFunctionType;
import org.apache.pinot.spi.config.table.ColumnPartitionConfig;
+import org.apache.pinot.spi.config.table.SegmentPartitionConfig;
import org.apache.pinot.spi.config.table.TableConfig;
import org.apache.pinot.spi.data.DateTimeFieldSpec;
-import org.apache.pinot.spi.data.DateTimeFormatSpec;
-import org.apache.pinot.spi.data.FieldSpec.DataType;
import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.utils.TimeUtils;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Review comment:
can you update the javadoc right on top here? This still lists the steps as per the old way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158#discussion_r670037610
##########
File path: pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/realtime_to_offline_segments/RealtimeToOfflineSegmentsTaskExecutor.java
##########
@@ -26,33 +26,31 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.TimeUnit;
import org.apache.commons.io.FileUtils;
import org.apache.helix.ZNRecord;
import org.apache.pinot.common.metadata.segment.SegmentZKMetadataCustomMapModifier;
import org.apache.pinot.common.minion.RealtimeToOfflineSegmentsTaskMetadata;
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.core.segment.processing.filter.RecordFilterConfig;
-import org.apache.pinot.core.segment.processing.filter.RecordFilterFactory;
import org.apache.pinot.core.segment.processing.framework.MergeType;
import org.apache.pinot.core.segment.processing.framework.SegmentConfig;
import org.apache.pinot.core.segment.processing.framework.SegmentProcessorConfig;
import org.apache.pinot.core.segment.processing.framework.SegmentProcessorFramework;
import org.apache.pinot.core.segment.processing.partitioner.PartitionerConfig;
import org.apache.pinot.core.segment.processing.partitioner.PartitionerFactory;
-import org.apache.pinot.core.segment.processing.transformer.RecordTransformerConfig;
+import org.apache.pinot.core.segment.processing.timehandler.TimeHandler;
+import org.apache.pinot.core.segment.processing.timehandler.TimeHandlerConfig;
import org.apache.pinot.minion.executor.MinionTaskZkMetadataManager;
import org.apache.pinot.plugin.minion.tasks.BaseMultipleSegmentsConversionExecutor;
import org.apache.pinot.plugin.minion.tasks.SegmentConversionResult;
import org.apache.pinot.segment.spi.AggregationFunctionType;
import org.apache.pinot.spi.config.table.ColumnPartitionConfig;
+import org.apache.pinot.spi.config.table.SegmentPartitionConfig;
import org.apache.pinot.spi.config.table.TableConfig;
import org.apache.pinot.spi.data.DateTimeFieldSpec;
-import org.apache.pinot.spi.data.DateTimeFormatSpec;
-import org.apache.pinot.spi.data.FieldSpec.DataType;
import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.utils.TimeUtils;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Review comment:
Good catch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158#issuecomment-878690453
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7158](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (72cd1ed) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/de2f0e04dca8130a09ea902787a75997b70cc16d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (de2f0e0) will **increase** coverage by `0.08%`.
> The diff coverage is `89.47%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7158 +/- ##
============================================
+ Coverage 73.49% 73.58% +0.08%
Complexity 92 92
============================================
Files 1500 1505 +5
Lines 73756 73799 +43
Branches 10637 10644 +7
============================================
+ Hits 54207 54302 +95
+ Misses 16012 15960 -52
Partials 3537 3537
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `41.95% <63.15%> (+0.04%)` | :arrow_up: |
| unittests | `65.27% <87.50%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../org/apache/pinot/core/common/MinionConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vTWluaW9uQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (ø)` | |
| [...processing/partitioner/ColumnValuePartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvQ29sdW1uVmFsdWVQYXJ0aXRpb25lci5qYXZh) | `100.00% <ø> (ø)` | |
| [...egment/processing/partitioner/NoOpPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvTm9PcFBhcnRpdGlvbmVyLmphdmE=) | `100.00% <ø> (ø)` | |
| [...ent/processing/partitioner/PartitionerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvUGFydGl0aW9uZXJGYWN0b3J5LmphdmE=) | `96.00% <ø> (ø)` | |
| [.../processing/partitioner/RoundRobinPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvUm91bmRSb2JpblBhcnRpdGlvbmVyLmphdmE=) | `100.00% <ø> (ø)` | |
| [...processing/partitioner/TableConfigPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvVGFibGVDb25maWdQYXJ0aXRpb25lci5qYXZh) | `100.00% <ø> (ø)` | |
| [...sing/partitioner/TransformFunctionPartitioner.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvcGFydGl0aW9uZXIvVHJhbnNmb3JtRnVuY3Rpb25QYXJ0aXRpb25lci5qYXZh) | `100.00% <ø> (ø)` | |
| [...ent/processing/timehandler/TimeHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvdGltZWhhbmRsZXIvVGltZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `73.33% <73.33%> (ø)` | |
| [.../core/segment/processing/mapper/SegmentMapper.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvbWFwcGVyL1NlZ21lbnRNYXBwZXIuamF2YQ==) | `88.13% <78.57%> (+1.46%)` | :arrow_up: |
| [...gments/RealtimeToOfflineSegmentsTaskGenerator.java](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvcmVhbHRpbWVfdG9fb2ZmbGluZV9zZWdtZW50cy9SZWFsdGltZVRvT2ZmbGluZVNlZ21lbnRzVGFza0dlbmVyYXRvci5qYXZh) | `93.38% <78.94%> (-1.28%)` | :arrow_down: |
| ... and [42 more](https://codecov.io/gh/apache/incubator-pinot/pull/7158/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [de2f0e0...72cd1ed](https://codecov.io/gh/apache/incubator-pinot/pull/7158?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #7158: Extract time handling for SegmentProcessorFramework
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7158:
URL: https://github.com/apache/incubator-pinot/pull/7158#discussion_r669935391
##########
File path: pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/realtime_to_offline_segments/RealtimeToOfflineSegmentsTaskExecutor.java
##########
@@ -132,67 +130,66 @@ public void preProcess(PinotTaskConfig pinotTaskConfig) {
String offlineTableName = TableNameBuilder.OFFLINE.tableNameWithType(rawTableName);
TableConfig tableConfig = getTableConfig(offlineTableName);
Schema schema = getSchema(offlineTableName);
- Set<String> schemaColumns = schema.getPhysicalColumnNames();
String timeColumn = tableConfig.getValidationConfig().getTimeColumnName();
DateTimeFieldSpec dateTimeFieldSpec = schema.getSpecForTimeColumn(timeColumn);
Preconditions
.checkState(dateTimeFieldSpec != null, "No valid spec found for time column: %s in schema for table: %s",
timeColumn, offlineTableName);
+ SegmentProcessorConfig.Builder segmentProcessorConfigBuilder =
+ new SegmentProcessorConfig.Builder().setTableConfig(tableConfig).setSchema(schema);
- long windowStartMs = Long.parseLong(configs.get(MinionConstants.RealtimeToOfflineSegmentsTask.WINDOW_START_MS_KEY));
- long windowEndMs = Long.parseLong(configs.get(MinionConstants.RealtimeToOfflineSegmentsTask.WINDOW_END_MS_KEY));
+ long windowStartMs = Long.parseLong(configs.get(RealtimeToOfflineSegmentsTask.WINDOW_START_MS_KEY));
+ long windowEndMs = Long.parseLong(configs.get(RealtimeToOfflineSegmentsTask.WINDOW_END_MS_KEY));
_nextWatermark = windowEndMs;
- String timeColumnTransformFunction =
Review comment:
Yes. Since rollup is not supported yet, decide to remove this config and mark it as backward-incompatible. The changes are listed within the release note.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org