You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/03/15 15:36:44 UTC

[GitHub] [arrow] lidavidm opened a new pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

lidavidm opened a new pull request #12636:
URL: https://github.com/apache/arrow/pull/12636


   Clarify that an empty list of locations means we should fetch data from the same server, and that multiple locations mean that all locations are equivalent.
   
   See ["[Java] [Flight] Usage of Locations on FlightEndpoint"](https://lists.apache.org/thread/1668fk1myqf8168xh296qck5fn8ztcmn) on user@ for the original discussion.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] lidavidm commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r827945977



##########
File path: java/flight/flight-integration-tests/src/main/java/org/apache/arrow/flight/integration/tests/IntegrationTestClient.java
##########
@@ -152,46 +153,57 @@ public void onNext(PutResult val) {
       // 3. Download the data from the server.
       List<Location> locations = endpoint.getLocations();
       if (locations.isEmpty()) {
-        throw new RuntimeException("No locations returned from Flight server.");
+        // No locations provided, validate the server itself.
+        System.out.println("Verifying location " + server.getUri());

Review comment:
       Removed, here and below.




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] github-actions[bot] commented on pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#issuecomment-1068175818


   https://issues.apache.org/jira/browse/ARROW-15921


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] BryanCutler commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
BryanCutler commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r830190249



##########
File path: format/Flight.proto
##########
@@ -259,8 +259,10 @@ message FlightInfo {
   FlightDescriptor flight_descriptor = 2;
 
   /*
-   * A list of endpoints associated with the flight. To consume the whole
-   * flight, all endpoints must be consumed.
+   * A list of endpoints associated with the flight. To consume the
+   * whole flight, all endpoints (and hence all Tickets) must be consumed.
+   *
+   * In other words, multiple endpoints provide partitioning.

Review comment:
       What I meant was that Flight doesn't have built-in logic to actively partition a given data set. It's up to the service implementation to assign partitions to a given ticket.




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jacques-n commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
jacques-n commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r829402920



##########
File path: format/Flight.proto
##########
@@ -280,9 +281,16 @@ message FlightEndpoint {
   Ticket ticket = 1;
 
   /*
-   * A list of URIs where this ticket can be redeemed. If the list is
-   * empty, the expectation is that the ticket can only be redeemed on the
-   * current service where the ticket was generated.
+   * A list of URIs where this ticket can be redeemed.
+   *
+   * If the list is empty, the expectation is that the ticket can only
+   * be redeemed on the current service where the ticket was
+   * generated.
+   *
+   * If the list is not empty, the expectation is that the ticket can
+   * be redeemed at any of the locations, and that the data returned
+   * will be equivalent. In other words, multiple locations provide
+   * redundancy/load balancing.

Review comment:
       Yeah. The intention was, if there are locations, you *must* use one of the locations to redeem the ticket. One of the locations may be the current server.




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jacques-n commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
jacques-n commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r829397014



##########
File path: format/Flight.proto
##########
@@ -280,9 +281,16 @@ message FlightEndpoint {
   Ticket ticket = 1;
 
   /*
-   * A list of URIs where this ticket can be redeemed. If the list is
-   * empty, the expectation is that the ticket can only be redeemed on the
-   * current service where the ticket was generated.
+   * A list of URIs where this ticket can be redeemed.
+   *
+   * If the list is empty, the expectation is that the ticket can only
+   * be redeemed on the current service where the ticket was
+   * generated.
+   *
+   * If the list is not empty, the expectation is that the ticket can
+   * be redeemed at any of the locations, and that the data returned
+   * will be equivalent. In other words, multiple locations provide
+   * redundancy/load balancing.

Review comment:
       I think the reason the original might be thought of as ambiguous is that people are failing to see that tickets were conceived of as single use. If tickets are single use, once I go to one location and use the ticket, I can't then use the ticket at a second location. A flight must be a consumption of all tickets to be complete. I wonder if instead of expressing here, we should clarify elsewhere that a ticket is single use and reuse of ticket results in undefined behavior. Thoughts?




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] lidavidm commented on pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
lidavidm commented on pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#issuecomment-1074226736


   I filed ARROW-15987 for the CI failure (this happens to HEAD as well)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] lidavidm commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r829400024



##########
File path: format/Flight.proto
##########
@@ -280,9 +281,16 @@ message FlightEndpoint {
   Ticket ticket = 1;
 
   /*
-   * A list of URIs where this ticket can be redeemed. If the list is
-   * empty, the expectation is that the ticket can only be redeemed on the
-   * current service where the ticket was generated.
+   * A list of URIs where this ticket can be redeemed.
+   *
+   * If the list is empty, the expectation is that the ticket can only
+   * be redeemed on the current service where the ticket was
+   * generated.
+   *
+   * If the list is not empty, the expectation is that the ticket can
+   * be redeemed at any of the locations, and that the data returned
+   * will be equivalent. In other words, multiple locations provide
+   * redundancy/load balancing.

Review comment:
       I think that would be good to clarify, but that doesn't answer the question of, "if there are locations, can the ticket still be redeemed on the original server, or must it only be redeemed on one of the given locations"?




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] lidavidm commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r829438654



##########
File path: format/Flight.proto
##########
@@ -280,9 +281,16 @@ message FlightEndpoint {
   Ticket ticket = 1;
 
   /*
-   * A list of URIs where this ticket can be redeemed. If the list is
-   * empty, the expectation is that the ticket can only be redeemed on the
-   * current service where the ticket was generated.
+   * A list of URIs where this ticket can be redeemed.
+   *
+   * If the list is empty, the expectation is that the ticket can only
+   * be redeemed on the current service where the ticket was
+   * generated.
+   *
+   * If the list is not empty, the expectation is that the ticket can
+   * be redeemed at any of the locations, and that the data returned
+   * will be equivalent. In other words, multiple locations provide
+   * redundancy/load balancing.

Review comment:
       Thanks for the comments, I've updated this PR with both the `Ticket` and `locations` clarifications




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] lidavidm commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r829554864



##########
File path: format/Flight.proto
##########
@@ -259,8 +259,10 @@ message FlightInfo {
   FlightDescriptor flight_descriptor = 2;
 
   /*
-   * A list of endpoints associated with the flight. To consume the whole
-   * flight, all endpoints must be consumed.
+   * A list of endpoints associated with the flight. To consume the
+   * whole flight, all endpoints (and hence all Tickets) must be consumed.

Review comment:
       Updated.
   
   Though - that brings up the question of how to represent an ordered dataset. Is the expectation that (say) if results are sorted, then the endpoints should also be sorted?

##########
File path: format/Flight.proto
##########
@@ -259,8 +259,10 @@ message FlightInfo {
   FlightDescriptor flight_descriptor = 2;
 
   /*
-   * A list of endpoints associated with the flight. To consume the whole
-   * flight, all endpoints must be consumed.
+   * A list of endpoints associated with the flight. To consume the
+   * whole flight, all endpoints (and hence all Tickets) must be consumed.
+   *
+   * In other words, multiple endpoints provide partitioning.

Review comment:
       That's a very good point. I've reworded things here, how does it look now?




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] BryanCutler commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
BryanCutler commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r827449705



##########
File path: java/flight/flight-integration-tests/src/main/java/org/apache/arrow/flight/integration/tests/IntegrationTestClient.java
##########
@@ -152,46 +153,57 @@ public void onNext(PutResult val) {
       // 3. Download the data from the server.
       List<Location> locations = endpoint.getLocations();
       if (locations.isEmpty()) {
-        throw new RuntimeException("No locations returned from Flight server.");
+        // No locations provided, validate the server itself.
+        System.out.println("Verifying location " + server.getUri());
+        testTicket(allocator, client, endpoint.getTicket(), inputPath);
+      } else {
+        // All locations should be equivalent, validate each one.
+        for (Location location : locations) {
+          System.out.println("Verifying location " + location.getUri());

Review comment:
       here also?

##########
File path: format/Flight.proto
##########
@@ -280,9 +281,16 @@ message FlightEndpoint {
   Ticket ticket = 1;
 
   /*
-   * A list of URIs where this ticket can be redeemed. If the list is
-   * empty, the expectation is that the ticket can only be redeemed on the
-   * current service where the ticket was generated.
+   * A list of URIs where this ticket can be redeemed.
+   *
+   * If the list is empty, the expectation is that the ticket can only
+   * be redeemed on the current service where the ticket was
+   * generated.
+   *
+   * If the list is not empty, the expectation is that the ticket can
+   * be redeemed at any of the locations, and that the data returned
+   * will be equivalent. In other words, multiple locations provide
+   * redundancy/load balancing.

Review comment:
       Is there an intention that if the endpoint list is not empty, the ticket must be redeemed from one of those and not the service that generated the ticket, or is that up to interpretation by the application?

##########
File path: java/flight/flight-integration-tests/src/main/java/org/apache/arrow/flight/integration/tests/IntegrationTestClient.java
##########
@@ -152,46 +153,57 @@ public void onNext(PutResult val) {
       // 3. Download the data from the server.
       List<Location> locations = endpoint.getLocations();
       if (locations.isEmpty()) {
-        throw new RuntimeException("No locations returned from Flight server.");
+        // No locations provided, validate the server itself.
+        System.out.println("Verifying location " + server.getUri());

Review comment:
       Was this intentional to leave in?




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] alamb commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r833679568



##########
File path: format/Flight.proto
##########
@@ -280,9 +287,20 @@ message FlightEndpoint {
   Ticket ticket = 1;
 
   /*
-   * A list of URIs where this ticket can be redeemed. If the list is
-   * empty, the expectation is that the ticket can only be redeemed on the
-   * current service where the ticket was generated.
+   * A list of URIs where this ticket can be redeemed via DoGet().
+   *
+   * If the list is empty, the expectation is that the ticket can only
+   * be redeemed on the current service where the ticket was
+   * generated.
+   *
+   * If the list is not empty, the expectation is that the ticket can
+   * be redeemed at any of the locations, and that the data returned
+   * will be equivalent. In this case, the ticket may only be redeemed
+   * at one of the given locations, and not (necessarily) on the
+   * current service.
+   *
+   * In other words, an application can use multiple locations to

Review comment:
       this clarification is very helpful 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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot commented on pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#issuecomment-1078139865


   Benchmark runs are scheduled for baseline = 012ae6e961dbb472c7862f40be5dc972a9bd3e91 and contender = a17137f37f50fc5824b56b2d61bee49c2a30dbb7. a17137f37f50fc5824b56b2d61bee49c2a30dbb7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/61e1f89868d84af2a29a25205fb61566...96eaeb3713e54cad804b464791904af4/)
   [Scheduled] [test-mac-arm](https://conbench.ursa.dev/compare/runs/16fb478b4eae41549f8a4870e231d6e5...b1ca728e1e4147119bf310709bd2b4a0/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/5c1253ea129847599efb4883a7d3011f...96fc960b134d44f483278f721455ac3a/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/44af97b1838843e7a1f79c4220f72acb...920d94c416784f9e93daa3957c07dde5/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] pitrou commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r831122417



##########
File path: format/Flight.proto
##########
@@ -298,6 +316,9 @@ message Location {
 /*
  * An opaque identifier that the service can use to retrieve a particular
  * portion of a stream.
+ *
+ * Tickets are meant to be single use. It is an error/undefined behavior

Review comment:
       "undefined behavior" sounds a bit scary given the baggage this term has in C/C++. Perhaps say that the behavior can be implement-dependent and that implementations are allowed to raise an error?

##########
File path: format/Flight.proto
##########
@@ -280,9 +287,20 @@ message FlightEndpoint {
   Ticket ticket = 1;
 
   /*
-   * A list of URIs where this ticket can be redeemed. If the list is
-   * empty, the expectation is that the ticket can only be redeemed on the
-   * current service where the ticket was generated.
+   * A list of URIs where this ticket can be redeemed.

Review comment:
       Perhaps also explain how someone is supposed to "redeem" a ticket (note that verb doesn't seem to exist in the RPC APIs)?




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] lidavidm commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r827945469



##########
File path: format/Flight.proto
##########
@@ -280,9 +281,16 @@ message FlightEndpoint {
   Ticket ticket = 1;
 
   /*
-   * A list of URIs where this ticket can be redeemed. If the list is
-   * empty, the expectation is that the ticket can only be redeemed on the
-   * current service where the ticket was generated.
+   * A list of URIs where this ticket can be redeemed.
+   *
+   * If the list is empty, the expectation is that the ticket can only
+   * be redeemed on the current service where the ticket was
+   * generated.
+   *
+   * If the list is not empty, the expectation is that the ticket can
+   * be redeemed at any of the locations, and that the data returned
+   * will be equivalent. In other words, multiple locations provide
+   * redundancy/load balancing.

Review comment:
       Ah, this is ambiguous still, but I think the former was intended. For instance from the [original Flight blog post](https://arrow.apache.org/blog/2019/10/13/introducing-arrow-flight/):
   
   > Nodes in a distributed cluster can take on different roles. For example, a subset of nodes might be responsible for planning queries while other nodes exclusively fulfill data stream (DoGet or DoPut) requests.
   
   @jacques-n what do you think?




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] BryanCutler commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
BryanCutler commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r829495259



##########
File path: format/Flight.proto
##########
@@ -280,9 +281,16 @@ message FlightEndpoint {
   Ticket ticket = 1;
 
   /*
-   * A list of URIs where this ticket can be redeemed. If the list is
-   * empty, the expectation is that the ticket can only be redeemed on the
-   * current service where the ticket was generated.
+   * A list of URIs where this ticket can be redeemed.
+   *
+   * If the list is empty, the expectation is that the ticket can only
+   * be redeemed on the current service where the ticket was
+   * generated.
+   *
+   * If the list is not empty, the expectation is that the ticket can
+   * be redeemed at any of the locations, and that the data returned
+   * will be equivalent. In other words, multiple locations provide
+   * redundancy/load balancing.

Review comment:
       That sounds reasonable 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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jacques-n commented on pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
jacques-n commented on pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#issuecomment-1074236886


   Thanks for working through this @lidavidm!


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jacques-n commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
jacques-n commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r829397014



##########
File path: format/Flight.proto
##########
@@ -280,9 +281,16 @@ message FlightEndpoint {
   Ticket ticket = 1;
 
   /*
-   * A list of URIs where this ticket can be redeemed. If the list is
-   * empty, the expectation is that the ticket can only be redeemed on the
-   * current service where the ticket was generated.
+   * A list of URIs where this ticket can be redeemed.
+   *
+   * If the list is empty, the expectation is that the ticket can only
+   * be redeemed on the current service where the ticket was
+   * generated.
+   *
+   * If the list is not empty, the expectation is that the ticket can
+   * be redeemed at any of the locations, and that the data returned
+   * will be equivalent. In other words, multiple locations provide
+   * redundancy/load balancing.

Review comment:
       I think the reason the original might be thought of as ambiguous is that people are failing to see that tickets were conceived of as single use. If tickets are single use, once I go to one location and use the ticket, I can't then use the ticket at a second location. A flight must be a consumption of all tickets to be complete. I wonder if instead of expressing here, we should clarify elsewhere that a ticket is single use and reuse of ticket results in undefined behavior. Thoughts?

##########
File path: format/Flight.proto
##########
@@ -280,9 +281,16 @@ message FlightEndpoint {
   Ticket ticket = 1;
 
   /*
-   * A list of URIs where this ticket can be redeemed. If the list is
-   * empty, the expectation is that the ticket can only be redeemed on the
-   * current service where the ticket was generated.
+   * A list of URIs where this ticket can be redeemed.
+   *
+   * If the list is empty, the expectation is that the ticket can only
+   * be redeemed on the current service where the ticket was
+   * generated.
+   *
+   * If the list is not empty, the expectation is that the ticket can
+   * be redeemed at any of the locations, and that the data returned
+   * will be equivalent. In other words, multiple locations provide
+   * redundancy/load balancing.

Review comment:
       Yeah. The intention was, if there are locations, you *must* use one of the locations to redeem the ticket. One of the locations may be the current server.




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot edited a comment on pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#issuecomment-1078139865


   Benchmark runs are scheduled for baseline = 012ae6e961dbb472c7862f40be5dc972a9bd3e91 and contender = a17137f37f50fc5824b56b2d61bee49c2a30dbb7. a17137f37f50fc5824b56b2d61bee49c2a30dbb7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/61e1f89868d84af2a29a25205fb61566...96eaeb3713e54cad804b464791904af4/)
   [Finished :arrow_down:0.71% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/16fb478b4eae41549f8a4870e231d6e5...b1ca728e1e4147119bf310709bd2b4a0/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/5c1253ea129847599efb4883a7d3011f...96fc960b134d44f483278f721455ac3a/)
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/44af97b1838843e7a1f79c4220f72acb...920d94c416784f9e93daa3957c07dde5/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot edited a comment on pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#issuecomment-1078139865


   Benchmark runs are scheduled for baseline = 012ae6e961dbb472c7862f40be5dc972a9bd3e91 and contender = a17137f37f50fc5824b56b2d61bee49c2a30dbb7. a17137f37f50fc5824b56b2d61bee49c2a30dbb7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/61e1f89868d84af2a29a25205fb61566...96eaeb3713e54cad804b464791904af4/)
   [Scheduled] [test-mac-arm](https://conbench.ursa.dev/compare/runs/16fb478b4eae41549f8a4870e231d6e5...b1ca728e1e4147119bf310709bd2b4a0/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/5c1253ea129847599efb4883a7d3011f...96fc960b134d44f483278f721455ac3a/)
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/44af97b1838843e7a1f79c4220f72acb...920d94c416784f9e93daa3957c07dde5/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] lidavidm commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r830177217



##########
File path: format/Flight.proto
##########
@@ -259,8 +259,10 @@ message FlightInfo {
   FlightDescriptor flight_descriptor = 2;
 
   /*
-   * A list of endpoints associated with the flight. To consume the whole
-   * flight, all endpoints must be consumed.
+   * A list of endpoints associated with the flight. To consume the
+   * whole flight, all endpoints (and hence all Tickets) must be consumed.
+   *
+   * In other words, multiple endpoints provide partitioning.

Review comment:
       I suppose it's because an application can implement partitioning via Flight, but Bryan has seen people thinking this means that using Flight will automatically partition/distribute data. 




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] lidavidm commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r829400024



##########
File path: format/Flight.proto
##########
@@ -280,9 +281,16 @@ message FlightEndpoint {
   Ticket ticket = 1;
 
   /*
-   * A list of URIs where this ticket can be redeemed. If the list is
-   * empty, the expectation is that the ticket can only be redeemed on the
-   * current service where the ticket was generated.
+   * A list of URIs where this ticket can be redeemed.
+   *
+   * If the list is empty, the expectation is that the ticket can only
+   * be redeemed on the current service where the ticket was
+   * generated.
+   *
+   * If the list is not empty, the expectation is that the ticket can
+   * be redeemed at any of the locations, and that the data returned
+   * will be equivalent. In other words, multiple locations provide
+   * redundancy/load balancing.

Review comment:
       I think that would be good to clarify, but that doesn't answer the question of, "if there are locations, can the ticket still be redeemed on the original server, or must it only be redeemed on one of the given locations"?

##########
File path: format/Flight.proto
##########
@@ -280,9 +281,16 @@ message FlightEndpoint {
   Ticket ticket = 1;
 
   /*
-   * A list of URIs where this ticket can be redeemed. If the list is
-   * empty, the expectation is that the ticket can only be redeemed on the
-   * current service where the ticket was generated.
+   * A list of URIs where this ticket can be redeemed.
+   *
+   * If the list is empty, the expectation is that the ticket can only
+   * be redeemed on the current service where the ticket was
+   * generated.
+   *
+   * If the list is not empty, the expectation is that the ticket can
+   * be redeemed at any of the locations, and that the data returned
+   * will be equivalent. In other words, multiple locations provide
+   * redundancy/load balancing.

Review comment:
       Thanks for the comments, I've updated this PR with both the `Ticket` and `locations` clarifications

##########
File path: format/Flight.proto
##########
@@ -259,8 +259,10 @@ message FlightInfo {
   FlightDescriptor flight_descriptor = 2;
 
   /*
-   * A list of endpoints associated with the flight. To consume the whole
-   * flight, all endpoints must be consumed.
+   * A list of endpoints associated with the flight. To consume the
+   * whole flight, all endpoints (and hence all Tickets) must be consumed.

Review comment:
       Updated.
   
   Though - that brings up the question of how to represent an ordered dataset. Is the expectation that (say) if results are sorted, then the endpoints should also be sorted?

##########
File path: format/Flight.proto
##########
@@ -259,8 +259,10 @@ message FlightInfo {
   FlightDescriptor flight_descriptor = 2;
 
   /*
-   * A list of endpoints associated with the flight. To consume the whole
-   * flight, all endpoints must be consumed.
+   * A list of endpoints associated with the flight. To consume the
+   * whole flight, all endpoints (and hence all Tickets) must be consumed.
+   *
+   * In other words, multiple endpoints provide partitioning.

Review comment:
       That's a very good point. I've reworded things here, how does it look now?




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] lidavidm commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r830181523



##########
File path: format/Flight.proto
##########
@@ -259,8 +259,10 @@ message FlightInfo {
   FlightDescriptor flight_descriptor = 2;
 
   /*
-   * A list of endpoints associated with the flight. To consume the whole
-   * flight, all endpoints must be consumed.
+   * A list of endpoints associated with the flight. To consume the
+   * whole flight, all endpoints (and hence all Tickets) must be consumed.

Review comment:
       Pushed a short clarification.




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] lidavidm closed pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
lidavidm closed pull request #12636:
URL: https://github.com/apache/arrow/pull/12636


   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jduo commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
jduo commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r829543013



##########
File path: format/Flight.proto
##########
@@ -259,8 +259,10 @@ message FlightInfo {
   FlightDescriptor flight_descriptor = 2;
 
   /*
-   * A list of endpoints associated with the flight. To consume the whole
-   * flight, all endpoints must be consumed.
+   * A list of endpoints associated with the flight. To consume the
+   * whole flight, all endpoints (and hence all Tickets) must be consumed.

Review comment:
       Is it worth clarifying that the list of endpoints can be consumed in any order?




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot edited a comment on pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#issuecomment-1078139865


   Benchmark runs are scheduled for baseline = 012ae6e961dbb472c7862f40be5dc972a9bd3e91 and contender = a17137f37f50fc5824b56b2d61bee49c2a30dbb7. a17137f37f50fc5824b56b2d61bee49c2a30dbb7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/61e1f89868d84af2a29a25205fb61566...96eaeb3713e54cad804b464791904af4/)
   [Finished :arrow_down:0.71% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/16fb478b4eae41549f8a4870e231d6e5...b1ca728e1e4147119bf310709bd2b4a0/)
   [Failed :arrow_down:0.36% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/5c1253ea129847599efb4883a7d3011f...96fc960b134d44f483278f721455ac3a/)
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/44af97b1838843e7a1f79c4220f72acb...920d94c416784f9e93daa3957c07dde5/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jduo commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
jduo commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r830124630



##########
File path: format/Flight.proto
##########
@@ -259,8 +259,10 @@ message FlightInfo {
   FlightDescriptor flight_descriptor = 2;
 
   /*
-   * A list of endpoints associated with the flight. To consume the whole
-   * flight, all endpoints must be consumed.
+   * A list of endpoints associated with the flight. To consume the
+   * whole flight, all endpoints (and hence all Tickets) must be consumed.

Review comment:
       My thought here is that the backend should enforce ordering by grouping ordered data together under one ticket. The client can't process this data in parallel anyway.
   
   This would prevent exposing ordered fragments from separate locations though. It sort of makes sense for a query engine to have to collate data under a single node though.




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jacques-n commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
jacques-n commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r830175697



##########
File path: format/Flight.proto
##########
@@ -259,8 +259,10 @@ message FlightInfo {
   FlightDescriptor flight_descriptor = 2;
 
   /*
-   * A list of endpoints associated with the flight. To consume the whole
-   * flight, all endpoints must be consumed.
+   * A list of endpoints associated with the flight. To consume the
+   * whole flight, all endpoints (and hence all Tickets) must be consumed.
+   *
+   * In other words, multiple endpoints provide partitioning.

Review comment:
       I'm confused by your comment here a little bit. Flight *can* do partitioning. It is done by returning more than one ticket for a particular flight. (I agree that locations aren't about partitioning.)




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jduo commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
jduo commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r829543013



##########
File path: format/Flight.proto
##########
@@ -259,8 +259,10 @@ message FlightInfo {
   FlightDescriptor flight_descriptor = 2;
 
   /*
-   * A list of endpoints associated with the flight. To consume the whole
-   * flight, all endpoints must be consumed.
+   * A list of endpoints associated with the flight. To consume the
+   * whole flight, all endpoints (and hence all Tickets) must be consumed.

Review comment:
       Is it worth clarifying that the list of endpoints can be consumed in any order?




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] BryanCutler commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
BryanCutler commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r829505007



##########
File path: format/Flight.proto
##########
@@ -259,8 +259,10 @@ message FlightInfo {
   FlightDescriptor flight_descriptor = 2;
 
   /*
-   * A list of endpoints associated with the flight. To consume the whole
-   * flight, all endpoints must be consumed.
+   * A list of endpoints associated with the flight. To consume the
+   * whole flight, all endpoints (and hence all Tickets) must be consumed.
+   *
+   * In other words, multiple endpoints provide partitioning.

Review comment:
       Sorry to come back to this, but I've had people come to me thinking that Flight will do the partitioning for them. So we don't want to make it seem like that is what's happening. Maybe reword slightly to something like "In other words, multiple endpoints can be used with partitioned data."  or just "... multiple endpoints provide for partitioned data." ?




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] BryanCutler commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
BryanCutler commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r829495259



##########
File path: format/Flight.proto
##########
@@ -280,9 +281,16 @@ message FlightEndpoint {
   Ticket ticket = 1;
 
   /*
-   * A list of URIs where this ticket can be redeemed. If the list is
-   * empty, the expectation is that the ticket can only be redeemed on the
-   * current service where the ticket was generated.
+   * A list of URIs where this ticket can be redeemed.
+   *
+   * If the list is empty, the expectation is that the ticket can only
+   * be redeemed on the current service where the ticket was
+   * generated.
+   *
+   * If the list is not empty, the expectation is that the ticket can
+   * be redeemed at any of the locations, and that the data returned
+   * will be equivalent. In other words, multiple locations provide
+   * redundancy/load balancing.

Review comment:
       That sounds reasonable to me

##########
File path: format/Flight.proto
##########
@@ -259,8 +259,10 @@ message FlightInfo {
   FlightDescriptor flight_descriptor = 2;
 
   /*
-   * A list of endpoints associated with the flight. To consume the whole
-   * flight, all endpoints must be consumed.
+   * A list of endpoints associated with the flight. To consume the
+   * whole flight, all endpoints (and hence all Tickets) must be consumed.
+   *
+   * In other words, multiple endpoints provide partitioning.

Review comment:
       Sorry to come back to this, but I've had people come to me thinking that Flight will do the partitioning for them. So we don't want to make it seem like that is what's happening. Maybe reword slightly to something like "In other words, multiple endpoints can be used with partitioned data."  or just "... multiple endpoints provide for partitioned data." ?




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] lidavidm commented on pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
lidavidm commented on pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#issuecomment-1072685869


   @emkornfield, @pitrou: any thoughts on the Flight spec clarifications 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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jacques-n commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
jacques-n commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r830178439



##########
File path: format/Flight.proto
##########
@@ -259,8 +259,10 @@ message FlightInfo {
   FlightDescriptor flight_descriptor = 2;
 
   /*
-   * A list of endpoints associated with the flight. To consume the whole
-   * flight, all endpoints must be consumed.
+   * A list of endpoints associated with the flight. To consume the
+   * whole flight, all endpoints (and hence all Tickets) must be consumed.

Review comment:
       I agree with @jduo on this comment. If data must be ordered, at least for now, it should be exposed via single ticket/endpoint.




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] lidavidm commented on a change in pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#discussion_r830177542



##########
File path: format/Flight.proto
##########
@@ -259,8 +259,10 @@ message FlightInfo {
   FlightDescriptor flight_descriptor = 2;
 
   /*
-   * A list of endpoints associated with the flight. To consume the whole
-   * flight, all endpoints must be consumed.
+   * A list of endpoints associated with the flight. To consume the
+   * whole flight, all endpoints (and hence all Tickets) must be consumed.

Review comment:
       That makes sense. Though I think this is getting far out of scope of the original clarification now.




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot edited a comment on pull request #12636: ARROW-15921: [Format][FlightRPC][C++][Java] Clarify interpretation of FlightEndpoint.locations

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12636:
URL: https://github.com/apache/arrow/pull/12636#issuecomment-1078139865


   Benchmark runs are scheduled for baseline = 012ae6e961dbb472c7862f40be5dc972a9bd3e91 and contender = a17137f37f50fc5824b56b2d61bee49c2a30dbb7. a17137f37f50fc5824b56b2d61bee49c2a30dbb7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Failed :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/61e1f89868d84af2a29a25205fb61566...96eaeb3713e54cad804b464791904af4/)
   [Finished :arrow_down:0.71% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/16fb478b4eae41549f8a4870e231d6e5...b1ca728e1e4147119bf310709bd2b4a0/)
   [Failed :arrow_down:0.36% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/5c1253ea129847599efb4883a7d3011f...96fc960b134d44f483278f721455ac3a/)
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/44af97b1838843e7a1f79c4220f72acb...920d94c416784f9e93daa3957c07dde5/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org