You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "swaminathanmanish (via GitHub)" <gi...@apache.org> on 2023/04/18 00:53:34 UTC
[GitHub] [pinot] swaminathanmanish opened a new pull request, #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
swaminathanmanish opened a new pull request, #10630:
URL: https://github.com/apache/pinot/pull/10630
**Problem**
Segment replacement has 2 phases (start, end). The start phase takes in the "**from**" and "**to**" list of segments, to create a lineage record that's used in the the end phase. The end phase expects all segments in the "**to**" to be available. However it can happen that only a subset of segments get created between the start and end phase.
**Usecase** - While ingesting batch of files via minion tasks, we use segment replacement to do a global atomic switch to avoid inconsistencies during ingestion. We start with a list of "**to**" segments, however can end up with a subset of the original "to" segments list (eg: files can have empty records).
Today the code has validations when segments in the original "to" list are not available, during the end phase of the protocol.
**Solution**
Allow the endReplaceSegments API take in the list of "**to**" segments that actually gets generated/created.
Validate that this list is a subset of the original to List.
**Tests**
1) Tested for backwards compatibility (MergeRollupMinionClusterIntegrationTest)
2) Unit tests
3) Used Swagger to verify that the new parameter is taken by the API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] snleee merged pull request #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
Posted by "snleee (via GitHub)" <gi...@apache.org>.
snleee merged PR #10630:
URL: https://github.com/apache/pinot/pull/10630
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] snleee commented on a diff in pull request #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
Posted by "snleee (via GitHub)" <gi...@apache.org>.
snleee commented on code in PR #10630:
URL: https://github.com/apache/pinot/pull/10630#discussion_r1170849059
##########
pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/EndReplaceSegmentsRequest.java:
##########
@@ -0,0 +1,42 @@
+package org.apache.pinot.common.restlet.resources;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.Collections;
+import java.util.List;
+import javax.annotation.Nullable;
+
+/**
Review Comment:
Can we move this header to the top of the code? (above the `package org.apache.pinot....`
##########
pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/EndReplaceSegmentsRequest.java:
##########
@@ -0,0 +1,42 @@
+package org.apache.pinot.common.restlet.resources;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.Collections;
+import java.util.List;
+import javax.annotation.Nullable;
+
+/**
+ * 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.
+ */
+
+/**
+ * segmentsTo: Actual merged segments. Sometimes not all segments that are passed into
Review Comment:
I think that we can change the wordings without mentioning about `merged segments`. This has initially been implemented as part of solving `atomically swapping original segments into merged segments`.
But, this building block actually provides us the flexibility of swapping any `segmentsFrom` into `segmentsTo`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] swaminathanmanish commented on pull request #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
Posted by "swaminathanmanish (via GitHub)" <gi...@apache.org>.
swaminathanmanish commented on PR #10630:
URL: https://github.com/apache/pinot/pull/10630#issuecomment-1513562159
> I had a similar use case where we wanted to avoid a check if all the `to segments` will be online or not. We were planning to add a boolean flag to the endReplacementAPI call to skip the toSegments online check. cc @jtao15 can add more to this
Thanks for the context. Please let me know if this PR will handle your usecase as well. We still wanted to enforce the checks on toSegments, but provide the flexibility to pass in a subset of toSegments (cc @snleee).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] swaminathanmanish commented on a diff in pull request #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
Posted by "swaminathanmanish (via GitHub)" <gi...@apache.org>.
swaminathanmanish commented on code in PR #10630:
URL: https://github.com/apache/pinot/pull/10630#discussion_r1171592951
##########
pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/EndReplaceSegmentsRequest.java:
##########
@@ -0,0 +1,42 @@
+package org.apache.pinot.common.restlet.resources;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.Collections;
+import java.util.List;
+import javax.annotation.Nullable;
+
+/**
+ * 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.
+ */
+
+/**
+ * segmentsTo: Actual merged segments. Sometimes not all segments that are passed into
Review Comment:
Makes sense. Updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] swaminathanmanish commented on pull request #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
Posted by "swaminathanmanish (via GitHub)" <gi...@apache.org>.
swaminathanmanish commented on PR #10630:
URL: https://github.com/apache/pinot/pull/10630#issuecomment-1513576438
> @swaminathanmanish For me to understand the use case, is it possible to avoid generating empty files in your offline data source? I feel like it would be cleaner to avoid generating empty files instead of changing the protocol.
Sure, for our ingestion usecase, we want to do a global atomic switch (fromSegments to toSegments), for data consistency reasons. We plan to do this in the Minion task generator, where we have the old & new segment names, since we use fixed name generator. We initiate the "start phase" when tasks are created and "end phase" after all tasks have been executed, in the generator. However the files are read only in the executor and can be empty (empty rows). Hence we need to be able to provide a subset of toSegments for the end phase. Note that this is a subset from the original toSegment list.
Could you describe your concerns about this change ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] snleee commented on a diff in pull request #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
Posted by "snleee (via GitHub)" <gi...@apache.org>.
snleee commented on code in PR #10630:
URL: https://github.com/apache/pinot/pull/10630#discussion_r1171617038
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/SegmentConversionUtils.java:
##########
@@ -200,18 +201,21 @@ public static String startSegmentReplace(String tableNameWithType, String upload
}
}
- public static void endSegmentReplace(String tableNameWithType, String uploadURL, String segmentLineageEntryId,
- int socketTimeoutMs, @Nullable AuthProvider authProvider)
+ public static void endSegmentReplace(String tableNameWithType, String uploadURL,
Review Comment:
Let's add one extra function without `EndReplaceSegmentsRequest` to keep the backward compatibility
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] swaminathanmanish commented on pull request #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
Posted by "swaminathanmanish (via GitHub)" <gi...@apache.org>.
swaminathanmanish commented on PR #10630:
URL: https://github.com/apache/pinot/pull/10630#issuecomment-1512289056
@snleee - Please review. Thanks !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jtao15 commented on pull request #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
Posted by "jtao15 (via GitHub)" <gi...@apache.org>.
jtao15 commented on PR #10630:
URL: https://github.com/apache/pinot/pull/10630#issuecomment-1513574128
@GSharayu @swaminathanmanish I think our use case is different, we have a benchmarking service, and would like to use the HWs smarter for consistent push enabled table. The ideal is to create the table first, and push the segments before allocating the HWs for benchmarking. We would like to skip the externalview check or make the check configurable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] swaminathanmanish commented on a diff in pull request #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
Posted by "swaminathanmanish (via GitHub)" <gi...@apache.org>.
swaminathanmanish commented on code in PR #10630:
URL: https://github.com/apache/pinot/pull/10630#discussion_r1171692325
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/SegmentConversionUtils.java:
##########
@@ -200,18 +201,21 @@ public static String startSegmentReplace(String tableNameWithType, String upload
}
}
- public static void endSegmentReplace(String tableNameWithType, String uploadURL, String segmentLineageEntryId,
- int socketTimeoutMs, @Nullable AuthProvider authProvider)
+ public static void endSegmentReplace(String tableNameWithType, String uploadURL,
Review Comment:
Ok, added.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jtao15 commented on pull request #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
Posted by "jtao15 (via GitHub)" <gi...@apache.org>.
jtao15 commented on PR #10630:
URL: https://github.com/apache/pinot/pull/10630#issuecomment-1513565769
@swaminathanmanish For me to understand the use case, is it possible to avoid generating empty files in your offline data source? I feel like it would be cleaner to avoid generating empty files instead of changing the protocol.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jtao15 commented on pull request #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
Posted by "jtao15 (via GitHub)" <gi...@apache.org>.
jtao15 commented on PR #10630:
URL: https://github.com/apache/pinot/pull/10630#issuecomment-1513652680
@snleee I thought `endReplacementProtocol(lineageId, [] <- empty list);` will also update the segmentsTo in lineage entry as empty. Did I miss anything 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] [pinot] snleee commented on pull request #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
Posted by "snleee (via GitHub)" <gi...@apache.org>.
snleee commented on PR #10630:
URL: https://github.com/apache/pinot/pull/10630#issuecomment-1513603392
@jtao15 @GSharayu I think that your use case can be modeled as:
```
endReplacementProtocol(lineageId, null / or empty list);
```
The above will bypass the external view's existence check. How do you think? In that way, we can avoid adding another boolean 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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] swaminathanmanish commented on pull request #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
Posted by "swaminathanmanish (via GitHub)" <gi...@apache.org>.
swaminathanmanish commented on PR #10630:
URL: https://github.com/apache/pinot/pull/10630#issuecomment-1513700000
> @GSharayu @swaminathanmanish I think our use case is different, we have a benchmarking service, and would like to use the HWs smarter for consistent push enabled table. The ideal is to create the table first, and push the segments before allocating the HWs for benchmarking. We would like to skip the externalview check or make the check configurable.
> @snleee I thought `endReplacementProtocol(lineageId, [] <- empty list);` will also update the segmentsTo in lineage entry to empty. Did I miss anything here?
Yes you are right. The lineage entry will be updated with the new toSegments list. For your usecase, we want to just skip the waitForSegmentsBecomeOnline 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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] snleee commented on pull request #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
Posted by "snleee (via GitHub)" <gi...@apache.org>.
snleee commented on PR #10630:
URL: https://github.com/apache/pinot/pull/10630#issuecomment-1513792564
@jtao15 Yes, you're right. I didn't go over this PR in detail when I wrote the comment above. Yes, we need the separate flag (bypassing the external view check) for your use case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] GSharayu commented on pull request #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
Posted by "GSharayu (via GitHub)" <gi...@apache.org>.
GSharayu commented on PR #10630:
URL: https://github.com/apache/pinot/pull/10630#issuecomment-1513506064
I had a similar use case where we wanted to avoid a check if all the `to segments` will be online or not. We were planning to add a boolean flag to the endReplacementAPI call to skip the toSegments online check. cc @jtao15 can add more to this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10630:
URL: https://github.com/apache/pinot/pull/10630#issuecomment-1512348290
## [Codecov](https://codecov.io/gh/apache/pinot/pull/10630?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 [#10630](https://codecov.io/gh/apache/pinot/pull/10630?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (61acc1d) into [master](https://codecov.io/gh/apache/pinot/commit/178aa200a78d5ecebff163529c965739ac2594e4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (178aa20) will **decrease** coverage by `3.18%`.
> The diff coverage is `36.36%`.
```diff
@@ Coverage Diff @@
## master #10630 +/- ##
============================================
- Coverage 27.74% 24.56% -3.18%
+ Complexity 58 49 -9
============================================
Files 2090 2091 +1
Lines 112642 112656 +14
Branches 16987 16991 +4
============================================
- Hits 31247 27677 -3570
- Misses 78272 82079 +3807
+ Partials 3123 2900 -223
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `24.56% <36.36%> (+0.11%)` | :arrow_up: |
| integration2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/10630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/restlet/resources/EndReplaceSegmentsRequest.java](https://codecov.io/gh/apache/pinot/pull/10630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvRW5kUmVwbGFjZVNlZ21lbnRzUmVxdWVzdC5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/pinot/pull/10630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `39.38% <0.00%> (-3.74%)` | :arrow_down: |
| [...t/segment/local/utils/ConsistentDataPushUtils.java](https://codecov.io/gh/apache/pinot/pull/10630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9Db25zaXN0ZW50RGF0YVB1c2hVdGlscy5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...e/pinot/common/utils/FileUploadDownloadClient.java](https://codecov.io/gh/apache/pinot/pull/10630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvRmlsZVVwbG9hZERvd25sb2FkQ2xpZW50LmphdmE=) | `49.39% <40.00%> (-0.81%)` | :arrow_down: |
| [...ces/PinotSegmentUploadDownloadRestletResource.java](https://codecov.io/gh/apache/pinot/pull/10630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90U2VnbWVudFVwbG9hZERvd25sb2FkUmVzdGxldFJlc291cmNlLmphdmE=) | `50.20% <100.00%> (ø)` | |
| [.../tasks/BaseMultipleSegmentsConversionExecutor.java](https://codecov.io/gh/apache/pinot/pull/10630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvQmFzZU11bHRpcGxlU2VnbWVudHNDb252ZXJzaW9uRXhlY3V0b3IuamF2YQ==) | `82.03% <100.00%> (ø)` | |
| [...ot/plugin/minion/tasks/SegmentConversionUtils.java](https://codecov.io/gh/apache/pinot/pull/10630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvU2VnbWVudENvbnZlcnNpb25VdGlscy5qYXZh) | `76.71% <100.00%> (ø)` | |
... and [263 files with indirect coverage changes](https://codecov.io/gh/apache/pinot/pull/10630/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] swaminathanmanish commented on a diff in pull request #10630: Adding an parameter (toSegments) to the endSegmentReplacement API
Posted by "swaminathanmanish (via GitHub)" <gi...@apache.org>.
swaminathanmanish commented on code in PR #10630:
URL: https://github.com/apache/pinot/pull/10630#discussion_r1171589878
##########
pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/EndReplaceSegmentsRequest.java:
##########
@@ -0,0 +1,42 @@
+package org.apache.pinot.common.restlet.resources;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.Collections;
+import java.util.List;
+import javax.annotation.Nullable;
+
+/**
Review Comment:
> Can we move this header to the top of the code? (above the `package org.apache.pinot....`
ah thanks for catching !
--
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