You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "snleee (via GitHub)" <gi...@apache.org> on 2023/02/16 07:02:13 UTC
[GitHub] [pinot] snleee opened a new pull request, #10290: Add a tight retry block to update ZK during endSegmentReplacement
snleee opened a new pull request, #10290:
URL: https://github.com/apache/pinot/pull/10290
- Current implementation waits for all segments to become online.
- By the time we update ZK, it's highly likely that the znode is already changed if there is another replacement protocol is happening at the same time.
- This PR adds a tight retry block for updating ZNode for segment lineage to improve the success rate.
--
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 #10290: Add a tight retry block to update ZK during endSegmentReplacement
Posted by "jtao15 (via GitHub)" <gi...@apache.org>.
jtao15 commented on PR #10290:
URL: https://github.com/apache/pinot/pull/10290#issuecomment-1433974409
LGTM, thanks for making the 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 merged pull request #10290: Add a tight retry block to update ZK during endSegmentReplacement
Posted by "snleee (via GitHub)" <gi...@apache.org>.
snleee merged PR #10290:
URL: https://github.com/apache/pinot/pull/10290
--
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 #10290: Add a tight retry block to update ZK during endSegmentReplacement
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10290:
URL: https://github.com/apache/pinot/pull/10290#issuecomment-1432656221
# [Codecov](https://codecov.io/gh/apache/pinot/pull/10290?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 [#10290](https://codecov.io/gh/apache/pinot/pull/10290?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f65f428) into [master](https://codecov.io/gh/apache/pinot/commit/2f345da102b4b099fa47d67fcc793dc3266d072a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2f345da) will **decrease** coverage by `50.46%`.
> The diff coverage is `83.33%`.
```diff
@@ Coverage Diff @@
## master #10290 +/- ##
=============================================
- Coverage 64.17% 13.72% -50.46%
+ Complexity 5754 182 -5572
=============================================
Files 1961 1961
Lines 106915 106924 +9
Branches 16356 16356
=============================================
- Hits 68614 14676 -53938
- Misses 33291 91082 +57791
+ Partials 5010 1166 -3844
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests1 | `?` | |
| unittests2 | `13.72% <83.33%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/10290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/pinot/pull/10290?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==) | `62.53% <83.33%> (+0.07%)` | :arrow_up: |
| [...src/main/java/org/apache/pinot/sql/FilterKind.java](https://codecov.io/gh/apache/pinot/pull/10290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvRmlsdGVyS2luZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/10290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...in/java/org/apache/pinot/spi/utils/BytesUtils.java](https://codecov.io/gh/apache/pinot/pull/10290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQnl0ZXNVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/common/CustomObject.java](https://codecov.io/gh/apache/pinot/pull/10290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vQ3VzdG9tT2JqZWN0LmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...n/java/org/apache/pinot/core/data/table/Table.java](https://codecov.io/gh/apache/pinot/pull/10290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1RhYmxlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/10290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/10290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/spi/trace/BaseRecording.java](https://codecov.io/gh/apache/pinot/pull/10290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvQmFzZVJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/spi/trace/NoOpRecording.java](https://codecov.io/gh/apache/pinot/pull/10290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvTm9PcFJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1326 more](https://codecov.io/gh/apache/pinot/pull/10290?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] snleee commented on a diff in pull request #10290: Add a tight retry block to update ZK during endSegmentReplacement
Posted by "snleee (via GitHub)" <gi...@apache.org>.
snleee commented on code in PR #10290:
URL: https://github.com/apache/pinot/pull/10290#discussion_r1109165695
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3473,23 +3458,41 @@ public void endReplaceSegments(String tableNameWithType, String segmentLineageEn
return false;
}
- // Update lineage entry
- LineageEntry newLineageEntry =
- new LineageEntry(lineageEntry.getSegmentsFrom(), segmentsTo, LineageEntryState.COMPLETED,
- System.currentTimeMillis());
- segmentLineage.updateLineageEntry(segmentLineageEntryId, newLineageEntry);
+ // Start the tighter loop for ZK Update to avoid too frequent write failure.
+ DEFAULT_RETRY_POLICY.attempt(() -> {
+ // Fetch the segment lineage again
+ ZNRecord segmentLineageToUpdateZNRecord =
+ SegmentLineageAccessHelper.getSegmentLineageZNRecord(_propertyStore, tableNameWithType);
+ int expectedVersion = segmentLineageToUpdateZNRecord.getVersion();
+ SegmentLineage segmentLineageToUpdate = SegmentLineage.fromZNRecord(segmentLineageToUpdateZNRecord);
+ LineageEntry lineageEntryToUpdate = segmentLineageToUpdate.getLineageEntry(segmentLineageEntryId);
+
+ // Check if we need to proceed with updating the lineage entry based on its current state.
+ if (!shouldUpdateLineageEntry(tableNameWithType, segmentLineageEntryId, lineageEntry)) {
Review Comment:
updated as discussed offline
--
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] Jackie-Jiang commented on a diff in pull request #10290: Add a tight retry block to update ZK during endSegmentReplacement
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #10290:
URL: https://github.com/apache/pinot/pull/10290#discussion_r1109167277
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3433,31 +3433,15 @@ public String startReplaceSegments(String tableNameWithType, List<String> segmen
public void endReplaceSegments(String tableNameWithType, String segmentLineageEntryId) {
try {
DEFAULT_RETRY_POLICY.attempt(() -> {
- // Fetch the segment lineage metadata
- ZNRecord segmentLineageZNRecord =
- SegmentLineageAccessHelper.getSegmentLineageZNRecord(_propertyStore, tableNameWithType);
- Preconditions.checkState(segmentLineageZNRecord != null, "Failed to find segment lineage for table: %s",
- tableNameWithType);
- SegmentLineage segmentLineage = SegmentLineage.fromZNRecord(segmentLineageZNRecord);
- int expectedVersion = segmentLineageZNRecord.getVersion();
-
- // Look up the lineage entry based on the segment lineage entry id
+ // Fetch the segment lineage and look up the lineage entry based on the entry id.
+ SegmentLineage segmentLineage = SegmentLineageAccessHelper.getSegmentLineage(_propertyStore, tableNameWithType);
LineageEntry lineageEntry = segmentLineage.getLineageEntry(segmentLineageEntryId);
Preconditions.checkState(lineageEntry != null, "Failed to find entry id: %s from segment lineage for table: %s",
segmentLineageEntryId, tableNameWithType);
- // NO-OPS if the entry is already 'COMPLETED', reject if the entry is 'REVERTED'
- if (lineageEntry.getState() == LineageEntryState.COMPLETED) {
- LOGGER.info("Found lineage entry is already in COMPLETED status. (tableNameWithType = {}, "
- + "segmentLineageEntryId = {})", tableNameWithType, segmentLineageEntryId);
+ // Check if we need to proceed with updating the lineage entry based on the current state.
+ if (!shouldUpdateLineageEntry(tableNameWithType, segmentLineageEntryId, lineageEntry)) {
Review Comment:
（minor) Suggest reverting this part since it is not reused. I feel the existing way is more readable
--
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 #10290: Add a tight retry block to update ZK during endSegmentReplacement
Posted by "snleee (via GitHub)" <gi...@apache.org>.
snleee commented on code in PR #10290:
URL: https://github.com/apache/pinot/pull/10290#discussion_r1108889975
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3433,31 +3433,16 @@ public String startReplaceSegments(String tableNameWithType, List<String> segmen
public void endReplaceSegments(String tableNameWithType, String segmentLineageEntryId) {
try {
DEFAULT_RETRY_POLICY.attempt(() -> {
Review Comment:
Good point. We still need to keep the top level retry logic to be backward compatible.
- I have seen the cases where waiting for segments to become online takes ~30minutes (think about the refresh push for TBs of data).
- Previous implementation was doing the retry for 5 times + wait 10 minutes for each round so we will be successful.
- If we remove the top level, we will fail after 10min waiting time.
--
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] zhtaoxiang commented on a diff in pull request #10290: Add a tight retry block to update ZK during endSegmentReplacement
Posted by "zhtaoxiang (via GitHub)" <gi...@apache.org>.
zhtaoxiang commented on code in PR #10290:
URL: https://github.com/apache/pinot/pull/10290#discussion_r1108166124
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3433,31 +3433,16 @@ public String startReplaceSegments(String tableNameWithType, List<String> segmen
public void endReplaceSegments(String tableNameWithType, String segmentLineageEntryId) {
try {
DEFAULT_RETRY_POLICY.attempt(() -> {
Review Comment:
Do we still need this outer retry logic if the logic is retried when trying to update znode?
--
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] Jackie-Jiang commented on a diff in pull request #10290: Add a tight retry block to update ZK during endSegmentReplacement
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #10290:
URL: https://github.com/apache/pinot/pull/10290#discussion_r1108916745
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3473,23 +3458,41 @@ public void endReplaceSegments(String tableNameWithType, String segmentLineageEn
return false;
}
- // Update lineage entry
- LineageEntry newLineageEntry =
- new LineageEntry(lineageEntry.getSegmentsFrom(), segmentsTo, LineageEntryState.COMPLETED,
- System.currentTimeMillis());
- segmentLineage.updateLineageEntry(segmentLineageEntryId, newLineageEntry);
+ // Start the tighter loop for ZK Update to avoid too frequent write failure.
+ DEFAULT_RETRY_POLICY.attempt(() -> {
+ // Fetch the segment lineage again
+ ZNRecord segmentLineageToUpdateZNRecord =
+ SegmentLineageAccessHelper.getSegmentLineageZNRecord(_propertyStore, tableNameWithType);
+ int expectedVersion = segmentLineageToUpdateZNRecord.getVersion();
+ SegmentLineage segmentLineageToUpdate = SegmentLineage.fromZNRecord(segmentLineageToUpdateZNRecord);
+ LineageEntry lineageEntryToUpdate = segmentLineageToUpdate.getLineageEntry(segmentLineageEntryId);
+
+ // Check if we need to proceed with updating the lineage entry based on its current state.
+ if (!shouldUpdateLineageEntry(tableNameWithType, segmentLineageEntryId, lineageEntry)) {
Review Comment:
We don't want to do this check again. We should check if the new fetched lineage entry is the same as the previous one, and then set the new lineage entry and post it back
--
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 #10290: Add a tight retry block to update ZK during endSegmentReplacement
Posted by "snleee (via GitHub)" <gi...@apache.org>.
snleee commented on code in PR #10290:
URL: https://github.com/apache/pinot/pull/10290#discussion_r1109295110
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3433,31 +3433,15 @@ public String startReplaceSegments(String tableNameWithType, List<String> segmen
public void endReplaceSegments(String tableNameWithType, String segmentLineageEntryId) {
try {
DEFAULT_RETRY_POLICY.attempt(() -> {
- // Fetch the segment lineage metadata
- ZNRecord segmentLineageZNRecord =
- SegmentLineageAccessHelper.getSegmentLineageZNRecord(_propertyStore, tableNameWithType);
- Preconditions.checkState(segmentLineageZNRecord != null, "Failed to find segment lineage for table: %s",
- tableNameWithType);
- SegmentLineage segmentLineage = SegmentLineage.fromZNRecord(segmentLineageZNRecord);
- int expectedVersion = segmentLineageZNRecord.getVersion();
-
- // Look up the lineage entry based on the segment lineage entry id
+ // Fetch the segment lineage and look up the lineage entry based on the entry id.
+ SegmentLineage segmentLineage = SegmentLineageAccessHelper.getSegmentLineage(_propertyStore, tableNameWithType);
LineageEntry lineageEntry = segmentLineage.getLineageEntry(segmentLineageEntryId);
Preconditions.checkState(lineageEntry != null, "Failed to find entry id: %s from segment lineage for table: %s",
segmentLineageEntryId, tableNameWithType);
- // NO-OPS if the entry is already 'COMPLETED', reject if the entry is 'REVERTED'
- if (lineageEntry.getState() == LineageEntryState.COMPLETED) {
- LOGGER.info("Found lineage entry is already in COMPLETED status. (tableNameWithType = {}, "
- + "segmentLineageEntryId = {})", tableNameWithType, segmentLineageEntryId);
+ // Check if we need to proceed with updating the lineage entry based on the current state.
+ if (!shouldUpdateLineageEntry(tableNameWithType, segmentLineageEntryId, lineageEntry)) {
Review Comment:
reverted
--
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 #10290: Add a tight retry block to update ZK during endSegmentReplacement
Posted by "snleee (via GitHub)" <gi...@apache.org>.
snleee commented on PR #10290:
URL: https://github.com/apache/pinot/pull/10290#issuecomment-1434136396
@zhtaoxiang @Jackie-Jiang I use the tight loop only for `endReplaceSegment & revertReplaceSegment` because `startReplaceSegment` actually touches multiple entries (it can mark other entry to be reverted).
--
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 #10290: Add a tight retry block to update ZK during endSegmentReplacement
Posted by "snleee (via GitHub)" <gi...@apache.org>.
snleee commented on code in PR #10290:
URL: https://github.com/apache/pinot/pull/10290#discussion_r1108889975
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3433,31 +3433,16 @@ public String startReplaceSegments(String tableNameWithType, List<String> segmen
public void endReplaceSegments(String tableNameWithType, String segmentLineageEntryId) {
try {
DEFAULT_RETRY_POLICY.attempt(() -> {
Review Comment:
Good point. We still need to keep the top level retry logic to be backward compatible.
- I have seen the cases where waiting for segments to become online takes ~30minutes (refresh data push for TBs of data).
- Previous implementation was doing the retry for 5 times + wait 10 minutes for each round so we will be successful.
- If we remove the top level, we will fail after 10min waiting time.
--
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