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/02/04 22:17:28 UTC
[GitHub] [arrow] rok opened a new pull request #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
rok opened a new pull request #12344:
URL: https://github.com/apache/arrow/pull/12344
This is to resolve [ARROW-13409](https://issues.apache.org/jira/browse/ARROW-13409).
--
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] rok commented on a change in pull request #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
rok commented on a change in pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#discussion_r800652247
##########
File path: cpp/src/arrow/flight/server.h
##########
@@ -200,10 +201,11 @@ class ARROW_FLIGHT_EXPORT FlightServerBase {
int GotSignal() const;
/// \brief Shut down the server. Can be called from signal handler or another
- /// thread while Serve() blocks.
+ /// thread while Serve() blocks. Optionally a deadline can be set. Once the
+ /// the deadline expires all pending calls associated with the server will be
+ /// forcefully canceled.
Review comment:
```suggestion
/// thread while Serve() blocks. Optionally a deadline can be set. Once the
/// the deadline expires server will switch to shutdown mode. All pending
/// async calls associated with the server will be forcefully canceled.
/// Remaining running sync calls will block until complete.
```
--
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] rok commented on a change in pull request #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
rok commented on a change in pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#discussion_r800222029
##########
File path: cpp/src/arrow/flight/flight_test.cc
##########
@@ -374,6 +374,27 @@ TEST(TestFlight, ServeShutdown) {
}
}
+TEST(TestFlight, ServeShutdownWithDeadline) {
+ constexpr int kIterations = 10;
+ for (int i = 0; i < kIterations; i++) {
+ Location location;
+ std::unique_ptr<FlightServerBase> server = ExampleTestServer();
+
+ ASSERT_OK(Location::ForGrpcTcp("localhost", 0, &location));
+ FlightServerOptions options(location);
+ ASSERT_OK(server->Init(options));
+ ASSERT_GT(server->port(), 0);
+ std::thread t([&]() { ASSERT_OK(server->Serve()); });
Review comment:
Removed.
--
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 #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#issuecomment-1032640748
Benchmark runs are scheduled for baseline = ddc4671a1c67cf7f9aaf4a604468c6dc8b5da97b and contender = 5a0ebb8aaa2dfabca072d18d9bea53fe813f519b. 5a0ebb8aaa2dfabca072d18d9bea53fe813f519b is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Failed] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/cac06a14a0d3410591162f657817a4ae...4fa93758ae204b798450ac2daa6ec299/)
[Failed] [test-mac-arm](https://conbench.ursa.dev/compare/runs/85a70dafbe7c469383de991a0c4f766c...3fa016fb74ea48a0b9419d2aa30bb601/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/f070403903ca4354b68dd56cefc1bcdc...50fec79450a4424c96f71636e2fe178c/)
[Failed] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/e4c336cf5770472589d2800fbe427971...714ed4990b994b37886317ebea9e4f3a/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. 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] rok commented on pull request #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
rok commented on pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#issuecomment-1031569266
@lidavidm fixed the linting. Sorry about that.
--
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 #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#issuecomment-1032640748
Benchmark runs are scheduled for baseline = ddc4671a1c67cf7f9aaf4a604468c6dc8b5da97b and contender = 5a0ebb8aaa2dfabca072d18d9bea53fe813f519b. 5a0ebb8aaa2dfabca072d18d9bea53fe813f519b 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/cac06a14a0d3410591162f657817a4ae...4fa93758ae204b798450ac2daa6ec299/)
[Scheduled] [test-mac-arm](https://conbench.ursa.dev/compare/runs/85a70dafbe7c469383de991a0c4f766c...3fa016fb74ea48a0b9419d2aa30bb601/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/f070403903ca4354b68dd56cefc1bcdc...50fec79450a4424c96f71636e2fe178c/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/e4c336cf5770472589d2800fbe427971...714ed4990b994b37886317ebea9e4f3a/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. 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 #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#discussion_r800666799
##########
File path: cpp/src/arrow/flight/server.h
##########
@@ -200,10 +201,11 @@ class ARROW_FLIGHT_EXPORT FlightServerBase {
int GotSignal() const;
/// \brief Shut down the server. Can be called from signal handler or another
- /// thread while Serve() blocks.
+ /// thread while Serve() blocks. Optionally a deadline can be set. Once the
+ /// the deadline expires all pending calls associated with the server will be
+ /// forcefully canceled.
Review comment:
Sounds good 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] rok commented on a change in pull request #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
rok commented on a change in pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#discussion_r801599864
##########
File path: cpp/src/arrow/flight/flight_test.cc
##########
@@ -374,6 +374,24 @@ TEST(TestFlight, ServeShutdown) {
}
}
+TEST(TestFlight, ServeShutdownWithDeadline) {
+ constexpr int kIterations = 10;
+ for (int i = 0; i < kIterations; i++) {
+ Location location;
+ std::unique_ptr<FlightServerBase> server = ExampleTestServer();
+
+ ASSERT_OK(Location::ForGrpcTcp("localhost", 0, &location));
+ FlightServerOptions options(location);
+ ASSERT_OK(server->Init(options));
+ ASSERT_GT(server->port(), 0);
+
+ auto deadline = std::chrono::system_clock::now() + std::chrono::microseconds(10);
+
+ ASSERT_OK(server->Shutdown(&deadline));
+ ASSERT_OK(server->Wait());
+ }
Review comment:
```suggestion
Location location;
std::unique_ptr<FlightServerBase> server = ExampleTestServer();
ASSERT_OK(Location::ForGrpcTcp("localhost", 0, &location));
FlightServerOptions options(location);
ASSERT_OK(server->Init(options));
ASSERT_GT(server->port(), 0);
auto deadline = std::chrono::system_clock::now() + std::chrono::microseconds(10);
ASSERT_OK(server->Shutdown(&deadline));
ASSERT_OK(server->Wait());
```
--
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 #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#issuecomment-1032640748
Benchmark runs are scheduled for baseline = ddc4671a1c67cf7f9aaf4a604468c6dc8b5da97b and contender = 5a0ebb8aaa2dfabca072d18d9bea53fe813f519b. 5a0ebb8aaa2dfabca072d18d9bea53fe813f519b 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/cac06a14a0d3410591162f657817a4ae...4fa93758ae204b798450ac2daa6ec299/)
[Scheduled] [test-mac-arm](https://conbench.ursa.dev/compare/runs/85a70dafbe7c469383de991a0c4f766c...3fa016fb74ea48a0b9419d2aa30bb601/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/f070403903ca4354b68dd56cefc1bcdc...50fec79450a4424c96f71636e2fe178c/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/e4c336cf5770472589d2800fbe427971...714ed4990b994b37886317ebea9e4f3a/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. 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 #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#issuecomment-1032640748
Benchmark runs are scheduled for baseline = ddc4671a1c67cf7f9aaf4a604468c6dc8b5da97b and contender = 5a0ebb8aaa2dfabca072d18d9bea53fe813f519b. 5a0ebb8aaa2dfabca072d18d9bea53fe813f519b is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/cac06a14a0d3410591162f657817a4ae...4fa93758ae204b798450ac2daa6ec299/)
[Scheduled] [test-mac-arm](https://conbench.ursa.dev/compare/runs/85a70dafbe7c469383de991a0c4f766c...3fa016fb74ea48a0b9419d2aa30bb601/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/f070403903ca4354b68dd56cefc1bcdc...50fec79450a4424c96f71636e2fe178c/)
[Finished :arrow_down:0.22% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/e4c336cf5770472589d2800fbe427971...714ed4990b994b37886317ebea9e4f3a/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. 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] rok commented on a change in pull request #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
rok commented on a change in pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#discussion_r801592511
##########
File path: cpp/src/arrow/flight/flight_test.cc
##########
@@ -374,6 +374,24 @@ TEST(TestFlight, ServeShutdown) {
}
}
+TEST(TestFlight, ServeShutdownWithDeadline) {
+ constexpr int kIterations = 10;
+ for (int i = 0; i < kIterations; i++) {
Review comment:
I'm modeling this on the test above (`ServeShutdown`). Not sure if it's useful.
--
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 #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#issuecomment-1032640748
Benchmark runs are scheduled for baseline = ddc4671a1c67cf7f9aaf4a604468c6dc8b5da97b and contender = 5a0ebb8aaa2dfabca072d18d9bea53fe813f519b. 5a0ebb8aaa2dfabca072d18d9bea53fe813f519b is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/cac06a14a0d3410591162f657817a4ae...4fa93758ae204b798450ac2daa6ec299/)
[Scheduled] [test-mac-arm](https://conbench.ursa.dev/compare/runs/85a70dafbe7c469383de991a0c4f766c...3fa016fb74ea48a0b9419d2aa30bb601/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/f070403903ca4354b68dd56cefc1bcdc...50fec79450a4424c96f71636e2fe178c/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/e4c336cf5770472589d2800fbe427971...714ed4990b994b37886317ebea9e4f3a/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. 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 closed pull request #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
pitrou closed pull request #12344:
URL: https://github.com/apache/arrow/pull/12344
--
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 #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#discussion_r800055709
##########
File path: cpp/src/arrow/flight/flight_test.cc
##########
@@ -374,6 +374,27 @@ TEST(TestFlight, ServeShutdown) {
}
}
+TEST(TestFlight, ServeShutdownWithDeadline) {
+ constexpr int kIterations = 10;
+ for (int i = 0; i < kIterations; i++) {
+ Location location;
+ std::unique_ptr<FlightServerBase> server = ExampleTestServer();
+
+ ASSERT_OK(Location::ForGrpcTcp("localhost", 0, &location));
+ FlightServerOptions options(location);
+ ASSERT_OK(server->Init(options));
+ ASSERT_GT(server->port(), 0);
+ std::thread t([&]() { ASSERT_OK(server->Serve()); });
Review comment:
nit, but actually the server is running as soon as you Init() it - no need to call Serve
##########
File path: cpp/src/arrow/flight/server.h
##########
@@ -25,6 +25,7 @@
#include <string>
#include <utility>
#include <vector>
+#include <grpcpp/support/time.h>
Review comment:
We can't expose gRPC symbols publically.
##########
File path: cpp/src/arrow/flight/server.h
##########
@@ -25,6 +25,7 @@
#include <string>
#include <utility>
#include <vector>
+#include <grpcpp/support/time.h>
Review comment:
The Shutdown method is templated: https://grpc.github.io/grpc/cpp/classgrpc_1_1_server_interface.html#a644772d11101318f1424e595dee73ccf and converts things via [TimePoint](https://grpc.github.io/grpc/cpp/classgrpc_1_1_time_point.html) which states it'll take a std::chrono::system_clock::time_point.
--
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] rok commented on a change in pull request #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
rok commented on a change in pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#discussion_r800667966
##########
File path: cpp/src/arrow/flight/server.h
##########
@@ -200,10 +201,11 @@ class ARROW_FLIGHT_EXPORT FlightServerBase {
int GotSignal() const;
/// \brief Shut down the server. Can be called from signal handler or another
- /// thread while Serve() blocks.
+ /// thread while Serve() blocks. Optionally a deadline can be set. Once the
+ /// the deadline expires all pending calls associated with the server will be
+ /// forcefully canceled.
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] rok commented on a change in pull request #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
rok commented on a change in pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#discussion_r800222099
##########
File path: cpp/src/arrow/flight/server.h
##########
@@ -25,6 +25,7 @@
#include <string>
#include <utility>
#include <vector>
+#include <grpcpp/support/time.h>
Review comment:
Yeah, that makes way more sense. Changed, please review.
--
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] rok commented on a change in pull request #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
rok commented on a change in pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#discussion_r800652870
##########
File path: cpp/src/arrow/flight/server.h
##########
@@ -200,10 +201,11 @@ class ARROW_FLIGHT_EXPORT FlightServerBase {
int GotSignal() const;
/// \brief Shut down the server. Can be called from signal handler or another
- /// thread while Serve() blocks.
+ /// thread while Serve() blocks. Optionally a deadline can be set. Once the
+ /// the deadline expires all pending calls associated with the server will be
+ /// forcefully canceled.
Review comment:
Does this look ok @lidavidm ? I'm not sure if flight uses the sync/async language.
--
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] rok commented on a change in pull request #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
rok commented on a change in pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#discussion_r800666182
##########
File path: cpp/src/arrow/flight/server.h
##########
@@ -200,10 +201,11 @@ class ARROW_FLIGHT_EXPORT FlightServerBase {
int GotSignal() const;
/// \brief Shut down the server. Can be called from signal handler or another
- /// thread while Serve() blocks.
+ /// thread while Serve() blocks. Optionally a deadline can be set. Once the
+ /// the deadline expires all pending calls associated with the server will be
+ /// forcefully canceled.
Review comment:
Got it :). So how about:
```suggestion
/// thread while Serve() blocks. Optionally a deadline can be set. Once the
/// the deadline expires server will wait until remaining running calls
/// complete.
```
--
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 #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#issuecomment-1032640748
Benchmark runs are scheduled for baseline = ddc4671a1c67cf7f9aaf4a604468c6dc8b5da97b and contender = 5a0ebb8aaa2dfabca072d18d9bea53fe813f519b. 5a0ebb8aaa2dfabca072d18d9bea53fe813f519b is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/cac06a14a0d3410591162f657817a4ae...4fa93758ae204b798450ac2daa6ec299/)
[Finished :arrow_down:0.26% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/85a70dafbe7c469383de991a0c4f766c...3fa016fb74ea48a0b9419d2aa30bb601/)
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/f070403903ca4354b68dd56cefc1bcdc...50fec79450a4424c96f71636e2fe178c/)
[Finished :arrow_down:0.22% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/e4c336cf5770472589d2800fbe427971...714ed4990b994b37886317ebea9e4f3a/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. 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 #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#discussion_r801597612
##########
File path: cpp/src/arrow/flight/flight_test.cc
##########
@@ -374,6 +374,24 @@ TEST(TestFlight, ServeShutdown) {
}
}
+TEST(TestFlight, ServeShutdownWithDeadline) {
+ constexpr int kIterations = 10;
+ for (int i = 0; i < kIterations; i++) {
Review comment:
I don't think the so. The test above is a bit more involved.
--
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 #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#discussion_r801591118
##########
File path: cpp/src/arrow/flight/flight_test.cc
##########
@@ -374,6 +374,24 @@ TEST(TestFlight, ServeShutdown) {
}
}
+TEST(TestFlight, ServeShutdownWithDeadline) {
+ constexpr int kIterations = 10;
+ for (int i = 0; i < kIterations; i++) {
Review comment:
Is it actually useful to do this in a loop?
--
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 #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#discussion_r800660834
##########
File path: cpp/src/arrow/flight/server.h
##########
@@ -200,10 +201,11 @@ class ARROW_FLIGHT_EXPORT FlightServerBase {
int GotSignal() const;
/// \brief Shut down the server. Can be called from signal handler or another
- /// thread while Serve() blocks.
+ /// thread while Serve() blocks. Optionally a deadline can be set. Once the
+ /// the deadline expires all pending calls associated with the server will be
+ /// forcefully canceled.
Review comment:
Ah, sorry, Flight does not use async gRPC at all. So what I mean is basically, "even if the deadline expires, this method will wait for existing calls to finish", or "the deadline does nothing" (right now :slightly_smiling_face:)
--
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 #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
lidavidm commented on pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#issuecomment-1031544495
Looks like this just needs to be linted.
--
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] rok commented on a change in pull request #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
rok commented on a change in pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#discussion_r801600751
##########
File path: cpp/src/arrow/flight/flight_test.cc
##########
@@ -374,6 +374,24 @@ TEST(TestFlight, ServeShutdown) {
}
}
+TEST(TestFlight, ServeShutdownWithDeadline) {
+ constexpr int kIterations = 10;
+ for (int i = 0; i < kIterations; i++) {
Review comment:
Removed the loop.
--
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 #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#issuecomment-1032640748
Benchmark runs are scheduled for baseline = ddc4671a1c67cf7f9aaf4a604468c6dc8b5da97b and contender = 5a0ebb8aaa2dfabca072d18d9bea53fe813f519b. 5a0ebb8aaa2dfabca072d18d9bea53fe813f519b is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Failed] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/cac06a14a0d3410591162f657817a4ae...4fa93758ae204b798450ac2daa6ec299/)
[Scheduled] [test-mac-arm](https://conbench.ursa.dev/compare/runs/85a70dafbe7c469383de991a0c4f766c...3fa016fb74ea48a0b9419d2aa30bb601/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/f070403903ca4354b68dd56cefc1bcdc...50fec79450a4424c96f71636e2fe178c/)
[Failed] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/e4c336cf5770472589d2800fbe427971...714ed4990b994b37886317ebea9e4f3a/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. 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] github-actions[bot] commented on pull request #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#issuecomment-1030394375
https://issues.apache.org/jira/browse/ARROW-13409
--
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 #12344: ARROW-13409: [C++][FlightRPC] Expose server shutdown with deadline
Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #12344:
URL: https://github.com/apache/arrow/pull/12344#discussion_r800638766
##########
File path: cpp/src/arrow/flight/server.h
##########
@@ -200,10 +201,11 @@ class ARROW_FLIGHT_EXPORT FlightServerBase {
int GotSignal() const;
/// \brief Shut down the server. Can be called from signal handler or another
- /// thread while Serve() blocks.
+ /// thread while Serve() blocks. Optionally a deadline can be set. Once the
+ /// the deadline expires all pending calls associated with the server will be
+ /// forcefully canceled.
Review comment:
Because gRPC won't cancel pending _sync_ calls which is what Flight uses, this is a little misleading. (See docs: https://grpc.github.io/grpc/cpp/classgrpc_1_1_server_interface.html#a644772d11101318f1424e595dee73ccf)
--
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