You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "tibrewalpratik17 (via GitHub)" <gi...@apache.org> on 2024/02/02 08:21:32 UTC
[PR] Add round-robin logic during downloadSegmentFromPeer [pinot]
tibrewalpratik17 opened a new pull request, #12353:
URL: https://github.com/apache/pinot/pull/12353
label: `bugfix`
During downloadSegmentFromPeer : https://github.com/apache/pinot/blob/041e04078f5a94fca92c805a8db8fdf1f904a985/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java#L596-L598
We saw that in some scenarios, the download request went to the same peer instead of having multiple replicas.
Logs:
```
host1 WARN Download and move segment tableName__3__7940__20240201T1322Z from peer with scheme http failed. 2024-02-02T04:40:43.510+00:00
host1 WARN Caught exception while fetching segment from: http://host2:25602/segments/tableName_REALTIME/tableName__3__7940__20240201T1322Z to: /data/upinot/stream-pinot-server/dataDir/tableName_REALTIME/tmp/tmp-tableName__3__7940__20240201T1322Z.1706848841111/tableName__3__7940__20240201T1322Z.tar.gz 2024-02-02T04:40:43.504+00:00
host1 WARN Caught exception while downloading segment from: http://host2:25602/segments/tableName_REALTIME/tableName__3__7940__20240201T1322Z to: /data/upinot/stream-pinot-server/dataDir/tableName_REALTIME/tmp/tmp-tableName__3__7940__20240201T1322Z.1706848841111/tableName__3__7940__20240201T1322Z.tar.gz 2024-02-02T04:40:43.503+00:00
host1 INFO Received a request to download segment tableName__3__7940__20240201T1322Z for table tableName_REALTIME 2024-02-02T04:40:43.502+00:00
host1 WARN Caught exception while downloading segment from: http://host2:25602/segments/tableName_REALTIME/tableName__3__7940__20240201T1322Z to: /data/upinot/stream-pinot-server/dataDir/tableName_REALTIME/tmp/tmp-tableName__3__7940__20240201T1322Z.1706848841111/tableName__3__7940__20240201T1322Z.tar.gz 2024-02-02T04:40:41.583+00:00
host1 WARN Caught exception while fetching segment from: http://host2:25602/segments/tableName_REALTIME/tableName__3__7940__20240201T1322Z to: /data/upinot/stream-pinot-server/dataDir/tableName_REALTIME/tmp/tmp-tableName__3__7940__20240201T1322Z.1706848841111/tableName__3__7940__20240201T1322Z.tar.gz 2024-02-02T04:40:41.583+00:00
host2 INFO Received a request to download segment tableName__3__7940__20240201T1322Z for table tableName_REALTIME 2024-02-02T04:40:41.581+00:00
host1 WARN Caught exception while downloading segment from: http://host2:25602/segments/tableName_REALTIME/tableName__3__7940__20240201T1322Z to: /data/upinot/stream-pinot-server/dataDir/tableName_REALTIME/tmp/tmp-tableName__3__7940__20240201T1322Z.1706848841111/tableName__3__7940__20240201T1322Z.tar.gz 2024-02-02T04:40:41.273+00:00
host1 WARN Caught exception while fetching segment from: http://host2:25602/segments/tableName_REALTIME/tableName__3__7940__20240201T1322Z to: /data/upinot/stream-pinot-server/dataDir/tableName_REALTIME/tmp/tmp-tableName__3__7940__20240201T1322Z.1706848841111/tableName__3__7940__20240201T1322Z.tar.gz 2024-02-02T04:40:41.273+00:00
```
This patch adds a round-robin logic to ensure we hit other peers as well to download replica.
--
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
Re: [PR] Add round-robin logic during downloadSegmentFromPeer [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #12353:
URL: https://github.com/apache/pinot/pull/12353#discussion_r1480328028
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/RoundRobinURIProvider.java:
##########
@@ -54,6 +55,14 @@ public RoundRobinURIProvider(URI originalUri)
_index = new Random().nextInt(_uris.length);
}
+ public RoundRobinURIProvider(List<URI> uris) {
Review Comment:
+1 on also resolve the hostname to multiple IPs to follow the existing behavior. We can probably add a helper method to resolve a given URI to a list of URIs
--
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
Re: [PR] Add round-robin logic during downloadSegmentFromPeer [pinot]
Posted by "tibrewalpratik17 (via GitHub)" <gi...@apache.org>.
tibrewalpratik17 commented on PR #12353:
URL: https://github.com/apache/pinot/pull/12353#issuecomment-1924772595
cc @Jackie-Jiang
--
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
Re: [PR] Add round-robin logic during downloadSegmentFromPeer [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on PR #12353:
URL: https://github.com/apache/pinot/pull/12353#issuecomment-1942159132
@tibrewalpratik17 : one of the tests is failing here across re-runs: https://github.com/apache/pinot/blob/master/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/PartialUpsertTableRebalanceIntegrationTest.java#L199
Looks like rebalance is not converging, and the rebalance may be using Segment Fetcher which in turn may be using the RoundRobinURIProvider. Can you take a look?
--
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
Re: [PR] Add round-robin logic during downloadSegmentFromPeer [pinot]
Posted by "tibrewalpratik17 (via GitHub)" <gi...@apache.org>.
tibrewalpratik17 commented on code in PR #12353:
URL: https://github.com/apache/pinot/pull/12353#discussion_r1478260899
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/RoundRobinURIProvider.java:
##########
@@ -54,6 +55,14 @@ public RoundRobinURIProvider(URI originalUri)
_index = new Random().nextInt(_uris.length);
}
+ public RoundRobinURIProvider(List<URI> uris) {
Review Comment:
Was able to re-purpose most of the things here and the naming is close to what we exactly want! Maybe we can modify javadocs accordingly.
cc @Jackie-Jiang wdyt?
--
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
Re: [PR] Add round-robin logic during downloadSegmentFromPeer [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #12353:
URL: https://github.com/apache/pinot/pull/12353#discussion_r1478254217
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/RoundRobinURIProvider.java:
##########
@@ -54,6 +55,14 @@ public RoundRobinURIProvider(URI originalUri)
_index = new Random().nextInt(_uris.length);
}
+ public RoundRobinURIProvider(List<URI> uris) {
Review Comment:
javadocs say that this class is supposed to resolve the hostname to multiple IP addresses. This ctor breaks that definition.
--
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
Re: [PR] Add round-robin logic during downloadSegmentFromPeer [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #12353:
URL: https://github.com/apache/pinot/pull/12353#discussion_r1482243905
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/RoundRobinURIProvider.java:
##########
@@ -63,4 +78,33 @@ public URI next() {
_index = (_index + 1) % _uris.length;
return result;
}
+
+ public URI[] resolveHostToIPAddresses(URI originalUri)
+ throws UnknownHostException, URISyntaxException {
+ URI[] resolvedUris;
+ String hostName = originalUri.getHost();
+ if (InetAddresses.isInetAddress(hostName)) {
+ resolvedUris = new URI[]{originalUri};
+ } else {
+ // Resolve host name to IP addresses via DNS
+ InetAddress[] addresses = InetAddress.getAllByName(hostName);
+ resolvedUris = new URI[addresses.length];
+ URIBuilder uriBuilder = new URIBuilder(originalUri);
+ for (int i = 0; i < addresses.length; i++) {
+ String ip = addresses[i].getHostAddress();
+ resolvedUris[i] = uriBuilder.setHost(ip).build();
+ }
+ }
+ return resolvedUris;
+ }
+
+ public URI[] resolveHostsToIPAddresses(List<URI> originalUri)
+ throws UnknownHostException, URISyntaxException {
+ List<URI> resolvedUrisList = new ArrayList<>();
+ for (URI uri : originalUri) {
+ resolvedUrisList.addAll(List.of(resolveHostToIPAddresses(uri)));
+ }
+
+ return resolvedUrisList.toArray(new URI[0]);
Review Comment:
Can avoid conversion between array and list and just use List everywhere.
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/RoundRobinURIProvider.java:
##########
@@ -38,18 +41,30 @@ public class RoundRobinURIProvider {
public RoundRobinURIProvider(URI originalUri)
throws UnknownHostException, URISyntaxException {
- String hostName = originalUri.getHost();
- if (InetAddresses.isInetAddress(hostName)) {
+ this(originalUri, true);
+ }
+
+ public RoundRobinURIProvider(URI originalUri, boolean resolveHost)
+ throws UnknownHostException, URISyntaxException {
+ if (resolveHost) {
+ _uris = resolveHostToIPAddresses(originalUri);
+ } else {
_uris = new URI[]{originalUri};
+ }
+ _index = new Random().nextInt(_uris.length);
+ }
+
+ public RoundRobinURIProvider(List<URI> originalUris)
+ throws UnknownHostException, URISyntaxException {
+ this(originalUris, true);
+ }
+
+ public RoundRobinURIProvider(List<URI> originalUris, boolean resolveHost)
Review Comment:
opinionated: I think keeping only 1 ctor (this one) would be cleaner. Callers can pass in a singleton list where required.
--
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
Re: [PR] Add round-robin logic during downloadSegmentFromPeer [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #12353:
URL: https://github.com/apache/pinot/pull/12353#discussion_r1482240176
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/RoundRobinURIProvider.java:
##########
@@ -63,4 +78,33 @@ public URI next() {
_index = (_index + 1) % _uris.length;
return result;
}
+
+ public URI[] resolveHostToIPAddresses(URI originalUri)
Review Comment:
I think this and the following method shouldn't be public, since we are anyways taking these args as input. To be more precise: if we create an object of `RoundRobinURIProvider` with some URIs, then we shouldn't allow callers to bypass those URIs (they should simply call `#next`)
--
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
Re: [PR] Add round-robin logic during downloadSegmentFromPeer [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #12353:
URL: https://github.com/apache/pinot/pull/12353#discussion_r1483039473
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/RoundRobinURIProvider.java:
##########
@@ -23,44 +23,66 @@
import java.net.URI;
import java.net.URISyntaxException;
import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Random;
import org.apache.http.client.utils.URIBuilder;
/**
- * RoundRobinURIProvider accept a URI, try to resolve it into multiple URIs with IP address, and return a IP address URI
- * in a Round Robin way.
+ * RoundRobinURIProvider accept a list of URIs and whether to resolve them into multiple URIs with IP address.
+ * If resolveHost = true, it returns a IP address URI in a Round Robin way.
+ * If resolveHost = false, then it returns a URI in a Round Robin way.
*/
public class RoundRobinURIProvider {
- private final URI[] _uris;
+ private final List<URI> _uris;
private int _index;
- public RoundRobinURIProvider(URI originalUri)
+ public RoundRobinURIProvider(List<URI> originalUris, boolean resolveHost)
throws UnknownHostException, URISyntaxException {
+ if (resolveHost) {
+ _uris = resolveHostsToIPAddresses(originalUris);
+ } else {
+ _uris = originalUris;
Review Comment:
Recommend creating a copy of `originalUris` since callers could modify the passed arg
--
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
Re: [PR] Add round-robin logic during downloadSegmentFromPeer [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana merged PR #12353:
URL: https://github.com/apache/pinot/pull/12353
--
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
Re: [PR] Add round-robin logic during downloadSegmentFromPeer [pinot]
Posted by "tibrewalpratik17 (via GitHub)" <gi...@apache.org>.
tibrewalpratik17 commented on PR #12353:
URL: https://github.com/apache/pinot/pull/12353#issuecomment-1942365282
> @tibrewalpratik17 : one of the tests is failing here across re-runs: https://github.com/apache/pinot/blob/master/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/PartialUpsertTableRebalanceIntegrationTest.java#L199
Looks like rebalance is not converging, and the rebalance may be using Segment Fetcher which in turn may be using the RoundRobinURIProvider. Can you take a look?
Looks flaky to me! I rebased and re-ran the workflows. In the current run: https://github.com/apache/pinot/actions/runs/7891406566/job/21535679431?pr=12353, some other integration test failed with this unrelated reason where server-starter failed:
```
PeerDownloadLLCRealtimeClusterIntegrationTest.setUp:101->BaseRealtimeClusterIntegrationTest.setUp:51->startServer:115->ClusterTest.startServers:270->ClusterTest.startOneServer:278 » Runtime java.io.IOException: Failed to bind to address 0.0.0.0/0.0.0.0:8475
```
--
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
Re: [PR] Add round-robin logic during downloadSegmentFromPeer [pinot]
Posted by "tibrewalpratik17 (via GitHub)" <gi...@apache.org>.
tibrewalpratik17 commented on code in PR #12353:
URL: https://github.com/apache/pinot/pull/12353#discussion_r1482663033
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/RoundRobinURIProvider.java:
##########
@@ -63,4 +78,33 @@ public URI next() {
_index = (_index + 1) % _uris.length;
return result;
}
+
+ public URI[] resolveHostToIPAddresses(URI originalUri)
+ throws UnknownHostException, URISyntaxException {
+ URI[] resolvedUris;
+ String hostName = originalUri.getHost();
+ if (InetAddresses.isInetAddress(hostName)) {
+ resolvedUris = new URI[]{originalUri};
+ } else {
+ // Resolve host name to IP addresses via DNS
+ InetAddress[] addresses = InetAddress.getAllByName(hostName);
+ resolvedUris = new URI[addresses.length];
+ URIBuilder uriBuilder = new URIBuilder(originalUri);
+ for (int i = 0; i < addresses.length; i++) {
+ String ip = addresses[i].getHostAddress();
+ resolvedUris[i] = uriBuilder.setHost(ip).build();
+ }
+ }
+ return resolvedUris;
+ }
+
+ public URI[] resolveHostsToIPAddresses(List<URI> originalUri)
+ throws UnknownHostException, URISyntaxException {
+ List<URI> resolvedUrisList = new ArrayList<>();
+ for (URI uri : originalUri) {
+ resolvedUrisList.addAll(List.of(resolveHostToIPAddresses(uri)));
+ }
+
+ return resolvedUrisList.toArray(new URI[0]);
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add round-robin logic during downloadSegmentFromPeer [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #12353:
URL: https://github.com/apache/pinot/pull/12353#issuecomment-1923391691
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/12353?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `8 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`041e040`)](https://app.codecov.io/gh/apache/pinot/commit/041e04078f5a94fca92c805a8db8fdf1f904a985?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.73% compared to head [(`7843c8e`)](https://app.codecov.io/gh/apache/pinot/pull/12353?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.73%.
| [Files](https://app.codecov.io/gh/apache/pinot/pull/12353?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ache/pinot/common/utils/RoundRobinURIProvider.java](https://app.codecov.io/gh/apache/pinot/pull/12353?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvUm91bmRSb2JpblVSSVByb3ZpZGVyLmphdmE=) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12353?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...pinot/common/utils/fetcher/BaseSegmentFetcher.java](https://app.codecov.io/gh/apache/pinot/pull/12353?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvZmV0Y2hlci9CYXNlU2VnbWVudEZldGNoZXIuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12353?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #12353 +/- ##
=========================================
Coverage 61.73% 61.73%
- Complexity 1152 1153 +1
=========================================
Files 2424 2424
Lines 132526 132532 +6
Branches 20483 20484 +1
=========================================
+ Hits 81810 81820 +10
+ Misses 44720 44717 -3
+ Partials 5996 5995 -1
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/12353/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/12353/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (ø)` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/12353/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (ø)` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/12353/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (ø)` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/12353/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (ø)` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/12353/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.68% <0.00%> (+<0.01%)` | :arrow_up: |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/12353/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.61% <0.00%> (+<0.01%)` | :arrow_up: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/12353/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.70% <0.00%> (-0.01%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/12353/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.60% <0.00%> (+0.01%)` | :arrow_up: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/12353/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.73% <0.00%> (+<0.01%)` | :arrow_up: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/12353/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.73% <0.00%> (+<0.01%)` | :arrow_up: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/12353/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.93% <0.00%> (+0.01%)` | :arrow_up: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/12353/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `27.69% <0.00%> (-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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/12353?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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
Re: [PR] Add round-robin logic during downloadSegmentFromPeer [pinot]
Posted by "tibrewalpratik17 (via GitHub)" <gi...@apache.org>.
tibrewalpratik17 commented on code in PR #12353:
URL: https://github.com/apache/pinot/pull/12353#discussion_r1480491731
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/RoundRobinURIProvider.java:
##########
@@ -54,6 +55,14 @@ public RoundRobinURIProvider(URI originalUri)
_index = new Random().nextInt(_uris.length);
}
+ public RoundRobinURIProvider(List<URI> uris) {
Review Comment:
Updated. Please 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
Re: [PR] Add round-robin logic during downloadSegmentFromPeer [pinot]
Posted by "tibrewalpratik17 (via GitHub)" <gi...@apache.org>.
tibrewalpratik17 commented on code in PR #12353:
URL: https://github.com/apache/pinot/pull/12353#discussion_r1482662603
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/RoundRobinURIProvider.java:
##########
@@ -63,4 +78,33 @@ public URI next() {
_index = (_index + 1) % _uris.length;
return result;
}
+
+ public URI[] resolveHostToIPAddresses(URI originalUri)
Review Comment:
Ah my bad missed this! Updated!
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/RoundRobinURIProvider.java:
##########
@@ -38,18 +41,30 @@ public class RoundRobinURIProvider {
public RoundRobinURIProvider(URI originalUri)
throws UnknownHostException, URISyntaxException {
- String hostName = originalUri.getHost();
- if (InetAddresses.isInetAddress(hostName)) {
+ this(originalUri, true);
+ }
+
+ public RoundRobinURIProvider(URI originalUri, boolean resolveHost)
+ throws UnknownHostException, URISyntaxException {
+ if (resolveHost) {
+ _uris = resolveHostToIPAddresses(originalUri);
+ } else {
_uris = new URI[]{originalUri};
+ }
+ _index = new Random().nextInt(_uris.length);
+ }
+
+ public RoundRobinURIProvider(List<URI> originalUris)
+ throws UnknownHostException, URISyntaxException {
+ this(originalUris, true);
+ }
+
+ public RoundRobinURIProvider(List<URI> originalUris, boolean resolveHost)
Review Comment:
Updated to just 1 ctor.
--
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
Re: [PR] Add round-robin logic during downloadSegmentFromPeer [pinot]
Posted by "tibrewalpratik17 (via GitHub)" <gi...@apache.org>.
tibrewalpratik17 commented on code in PR #12353:
URL: https://github.com/apache/pinot/pull/12353#discussion_r1483220623
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/RoundRobinURIProvider.java:
##########
@@ -23,44 +23,66 @@
import java.net.URI;
import java.net.URISyntaxException;
import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Random;
import org.apache.http.client.utils.URIBuilder;
/**
- * RoundRobinURIProvider accept a URI, try to resolve it into multiple URIs with IP address, and return a IP address URI
- * in a Round Robin way.
+ * RoundRobinURIProvider accept a list of URIs and whether to resolve them into multiple URIs with IP address.
+ * If resolveHost = true, it returns a IP address URI in a Round Robin way.
+ * If resolveHost = false, then it returns a URI in a Round Robin way.
*/
public class RoundRobinURIProvider {
- private final URI[] _uris;
+ private final List<URI> _uris;
private int _index;
- public RoundRobinURIProvider(URI originalUri)
+ public RoundRobinURIProvider(List<URI> originalUris, boolean resolveHost)
throws UnknownHostException, URISyntaxException {
+ if (resolveHost) {
+ _uris = resolveHostsToIPAddresses(originalUris);
+ } else {
+ _uris = originalUris;
Review Comment:
Done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add round-robin logic during downloadSegmentFromPeer [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on PR #12353:
URL: https://github.com/apache/pinot/pull/12353#issuecomment-1934941633
@Jackie-Jiang : can you also take a look? 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