Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C++][Go][Java][FlightRPC] Add support for result set expiration #35500

Closed
Tracked by #36954
kou opened this issue May 9, 2023 · 7 comments · Fixed by #36009
Closed
Tracked by #36954

[C++][Go][Java][FlightRPC] Add support for result set expiration #35500

kou opened this issue May 9, 2023 · 7 comments · Fixed by #36009

Comments

@kou
Copy link
Member

kou commented May 9, 2023

Describe the enhancement requested

Based on the proposal in https://docs.google.com/document/d/1jhPyPZSOo2iy0LqIJVUs9KWPyFULVFJXTILDfkadx2g/edit# .
See also the discussion thread: https://lists.apache.org/thread/247z3t06mf132nocngc1jkp3oqglz7jp

Currently, it is undefined whether a client can call DoGet more than once. Clients may want to retry requests, and servers may not want to persist a query result forever.

Proposal

Add an expiration time to FlightEndpoint. If present, clients may assume they can retry DoGet requests. Otherwise, clients should avoid retrying DoGet requests.

message FlightEndpoint {
  // In UTC.
  google.protobuf.Timestamp expiration_time = 3;
}

This proposal is not a full retry protocol.

Also, add “pre-defined” actions to Flight RPC for working with result sets. These are pre-defined Protobuf messages with standardized encodings for use with DoAction:

  • CancelQuery CancelFlightInfo: Asynchronously cancel the execution of a distributed query. (Replaces the equivalent Flight SQL action.)
  • RefreshQuery RefreshFlightEndpoint: Request an extension of the expiration of a FlightEndpoint.
  • CloseQuery CloseFlightInfo: Close a FlightInfo so that the server can clean up resources early. (Only CancelFlightInfo will be enough.)

This lets the ADBC/JDBC/ODBC drivers for Flight SQL explicitly manage result set lifetimes. These can be used with Flight SQL as regular actions.

Prior Art

Component(s)

C++, FlightRPC

@kou
Copy link
Member Author

kou commented May 25, 2023

https://lists.apache.org/thread/71pp95q6yklodm6lfjttswr3slfowdrb

About CloseQuery:

maybe it would be nice to name it 'CloseFlightInfo', to be matched with GetFlightInfo.

I agree with this. I'll use CancelFlightInfo/RefreshFlightInfo/CloseFlightInfo.

@kou
Copy link
Member Author

kou commented Jun 6, 2023

Ah, should RefreshQuery extend a expiration time of a FlightEndpoint? Or all expiration times of FlightInfo?
If the former, we should use RefereshFlightEndpoint, RefreshFlightInfo otherwise.

@kou
Copy link
Member Author

kou commented Jun 6, 2023

RefreshFlightEndpoint will be better because it's difficult that we return a suitable response for refreshing multiple FlightEndpoints. (If one of multiple FlightEndpoints is only failed, what response is suitable?)

@lidavidm
Copy link
Member

lidavidm commented Jun 6, 2023

RefreshFlightEndpoint should be more flexible, agreed.

kou added a commit to kou/arrow that referenced this issue Jun 9, 2023
…ation

Currently, it is undefined whether a client can call DoGet more than
once. Clients may want to retry requests, and servers may not want to
persist a query result forever.

Add an expiration time to FlightEndpoint. If present, clients may
assume they can retry DoGet requests. Otherwise, clients should avoid
retrying DoGet requests.

This is not a full retry protocol.

Also, add "pre-defined" actions to Flight RPC for working with result
sets. These are pre-defined Protobuf messages with standardized
encodings for use with DoAction:

  * CancelFlightInfo: Asynchronously cancel the execution of a distributed query. (Replaces the equivalent Flight SQL action.)
  * RefreshFlightEndpoint: Request an extension of the expiration of a FlightEndpoint.
  * CloseFlightInfo: Close a FlightInfo so that the server can clean up resources early.

This lets the ADBC/JDBC/ODBC drivers for Flight SQL explicitly manage
result set lifetimes. These can be used with Flight SQL as regular
actions.
kou added a commit to kou/arrow that referenced this issue Jun 9, 2023
…ation

Currently, it is undefined whether a client can call DoGet more than
once. Clients may want to retry requests, and servers may not want to
persist a query result forever.

Add an expiration time to FlightEndpoint. If present, clients may
assume they can retry DoGet requests. Otherwise, clients should avoid
retrying DoGet requests.

This is not a full retry protocol.

Also, add "pre-defined" actions to Flight RPC for working with result
sets. These are pre-defined Protobuf messages with standardized
encodings for use with DoAction:

  * CancelFlightInfo: Asynchronously cancel the execution of a distributed query. (Replaces the equivalent Flight SQL action.)
  * RefreshFlightEndpoint: Request an extension of the expiration of a FlightEndpoint.
  * CloseFlightInfo: Close a FlightInfo so that the server can clean up resources early.

This lets the ADBC/JDBC/ODBC drivers for Flight SQL explicitly manage
result set lifetimes. These can be used with Flight SQL as regular
actions.
kou added a commit to kou/arrow that referenced this issue Jun 19, 2023
…ation

Currently, it is undefined whether a client can call DoGet more than
once. Clients may want to retry requests, and servers may not want to
persist a query result forever.

Add an expiration time to FlightEndpoint. If present, clients may
assume they can retry DoGet requests. Otherwise, clients should avoid
retrying DoGet requests.

This is not a full retry protocol.

Also, add "pre-defined" actions to Flight RPC for working with result
sets. These are pre-defined Protobuf messages with standardized
encodings for use with DoAction:

  * CancelFlightInfo: Asynchronously cancel the execution of a distributed query. (Replaces the equivalent Flight SQL action.)
  * RefreshFlightEndpoint: Request an extension of the expiration of a FlightEndpoint.
  * CloseFlightInfo: Close a FlightInfo so that the server can clean up resources early.

This lets the ADBC/JDBC/ODBC drivers for Flight SQL explicitly manage
result set lifetimes. These can be used with Flight SQL as regular
actions.
@kou
Copy link
Member Author

kou commented Jun 19, 2023

Discussion:
[DISCUSS][Format][Flight] Result set expiration support
https://lists.apache.org/thread/48fqd554gkqrrld8k13l3b8trz5gk7ow

@kou kou changed the title [C++][FlightRPC] Add support for result set expiration [C++][Go][FlightRPC] Add support for result set expiration Jun 19, 2023
@kou
Copy link
Member Author

kou commented Jun 25, 2023

Changes based on discussion on the mailing list:

https://lists.apache.org/thread/ofw6bydhyhcqqn3t8013gx0k6fbw8lvv

  • Remove CloseFlightInfo (if nobody objects it)
  • RefreshFlightEndpoint ->
    RenewFlightEndpoint
  • RenewFlightEndpoint(FlightEndpoint) ->
    RenewFlightEndpoint(RenewFlightEndpointRequest)
  • CancelFlightInfo(FlightInfo) ->
    CancelFlightInfo(CancelFlightInfoRequest)

kou added a commit to kou/arrow that referenced this issue Jun 26, 2023
…ation

Currently, it is undefined whether a client can call DoGet more than
once. Clients may want to retry requests, and servers may not want to
persist a query result forever.

Add an expiration time to FlightEndpoint. If present, clients may
assume they can retry DoGet requests. Otherwise, clients should avoid
retrying DoGet requests.

This is not a full retry protocol.

Also, add "pre-defined" actions to Flight RPC for working with result
sets. These are pre-defined Protobuf messages with standardized
encodings for use with DoAction:

  * CancelFlightInfo: Asynchronously cancel the execution of a distributed query. (Replaces the equivalent Flight SQL action.)
  * RefreshFlightEndpoint: Request an extension of the expiration of a FlightEndpoint.
  * CloseFlightInfo: Close a FlightInfo so that the server can clean up resources early.

This lets the ADBC/JDBC/ODBC drivers for Flight SQL explicitly manage
result set lifetimes. These can be used with Flight SQL as regular
actions.
@kou
Copy link
Member Author

kou commented Jul 3, 2023

@kou kou closed this as completed in #36009 Jul 3, 2023
kou added a commit that referenced this issue Jul 3, 2023
…ation (#36009)

### Rationale for this change

Currently, it is undefined whether a client can call DoGet more than once. Clients may want to retry requests, and servers may not want to persist a query result forever.

### What changes are included in this PR?

Add an expiration time to FlightEndpoint. If present, clients may assume they can retry DoGet requests. Otherwise, clients should avoid retrying DoGet requests.

This is not a full retry protocol.

Also, add "pre-defined" actions to Flight RPC for working with result sets. These are pre-defined Protobuf messages with standardized encodings for use with DoAction:

  * CancelFlightInfo: Asynchronously cancel the execution of a distributed query. (Replaces the equivalent Flight SQL action.)
  * RefreshFlightEndpoint: Request an extension of the expiration of a FlightEndpoint.

This lets the ADBC/JDBC/ODBC drivers for Flight SQL explicitly manage result set lifetimes. These can be used with Flight SQL as regular actions.

#### Backward compatibility

Flight SQL's CancelQuery is deprecated by Flight RPC's CancelFlightInfo. But some clients may not be able to migrate to CancelFlightInfo entirely.

If a client can assume that a server requires 13.0.0 or later, the client can always use CancelFlightInfo. Otherwise, the client need to use CancelQuery (for old server) and/or CancelFlightInfo (for newer server).

A server needs to implement only CancelFlightInfo. Because Flight SQL server libraries provide the default CancelQuery implementation that delegates to CancelFlightInfo. Clients can use both of Flight SQL's CancelQuery and Flight RPC's CancelFLightInfo by this feature.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes.

* Closes: #35500

Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@kou kou added this to the 13.0.0 milestone Jul 3, 2023
@kou kou changed the title [C++][Go][FlightRPC] Add support for result set expiration [C++][Go][Java][FlightRPC] Add support for result set expiration Jul 3, 2023
westonpace pushed a commit to westonpace/arrow that referenced this issue Jul 7, 2023
… expiration (apache#36009)

### Rationale for this change

Currently, it is undefined whether a client can call DoGet more than once. Clients may want to retry requests, and servers may not want to persist a query result forever.

### What changes are included in this PR?

Add an expiration time to FlightEndpoint. If present, clients may assume they can retry DoGet requests. Otherwise, clients should avoid retrying DoGet requests.

This is not a full retry protocol.

Also, add "pre-defined" actions to Flight RPC for working with result sets. These are pre-defined Protobuf messages with standardized encodings for use with DoAction:

  * CancelFlightInfo: Asynchronously cancel the execution of a distributed query. (Replaces the equivalent Flight SQL action.)
  * RefreshFlightEndpoint: Request an extension of the expiration of a FlightEndpoint.

This lets the ADBC/JDBC/ODBC drivers for Flight SQL explicitly manage result set lifetimes. These can be used with Flight SQL as regular actions.

#### Backward compatibility

Flight SQL's CancelQuery is deprecated by Flight RPC's CancelFlightInfo. But some clients may not be able to migrate to CancelFlightInfo entirely.

If a client can assume that a server requires 13.0.0 or later, the client can always use CancelFlightInfo. Otherwise, the client need to use CancelQuery (for old server) and/or CancelFlightInfo (for newer server).

A server needs to implement only CancelFlightInfo. Because Flight SQL server libraries provide the default CancelQuery implementation that delegates to CancelFlightInfo. Clients can use both of Flight SQL's CancelQuery and Flight RPC's CancelFLightInfo by this feature.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes.

* Closes: apache#35500

Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants