Skip to content

ARROW-11717: [Integration] Fix intermittent flight integration failures with rust - #9593

Closed
alamb wants to merge 2 commits into
apache:masterfrom
alamb:alamb/arrow-11717
Closed

ARROW-11717: [Integration] Fix intermittent flight integration failures with rust#9593
alamb wants to merge 2 commits into
apache:masterfrom
alamb:alamb/arrow-11717

Conversation

@alamb

Copy link
Copy Markdown
Contributor

Background

Thanks to the 🦅 👁️ of @lidavidm#9583 (comment) it appears that the Rust flight integration test prints "Server listening" before the server is actually read to accept connections. Thus creating a race condition: if the tests try and start faster than the rust server can actually be setup then we see integration test failures as shown in https://issues.apache.org/jira/browse/ARROW-11717:

Traceback (most recent call last):
File "/arrow/dev/archery/archery/integration/runner.py", line 308, in _run_flight_test_case
consumer.flight_request(port, **client_args)
File "/arrow/dev/archery/archery/integration/tester_cpp.py", line 116, in flight_request
run_cmd(cmd)
File "/arrow/dev/archery/archery/integration/util.py", line 148, in run_cmd
raise RuntimeError(sio.getvalue())
RuntimeError: Command failed: /build/cpp/debug/flight-test-integration-client -host localhost -port=33569 -scenario auth:basic_proto
With output:
--------------
-- Arrow Fatal Error --
Invalid: Expected UNAUTHENTICATED but got Unavailable

Changes

This PR changes the Rust flight scenario harnesses to print the ready message after the server is actually ready.

@github-actions

Copy link
Copy Markdown

@lidavidm

Copy link
Copy Markdown
Member

@alamb if I'm not mistaken, serve().await? will block until the server is terminated, which means that the test is now stuck. Is there any way to start a server without waiting for it? (Most other gRPC implementations provide this.)

Alternatively, we could add a retry loop…

@alamb

Copy link
Copy Markdown
ContributorAuthor

@lidavidm good call -- let me try something else

@alamb

Copy link
Copy Markdown
ContributorAuthor

@lidavidm thank you for the help -- it is like you are writing the code for me :)

@alamb

Copy link
Copy Markdown
ContributorAuthor

The integration test passed: https://github.com/apache/arrow/pull/9593/checks?check_run_id=1995394206

I'll manually retrigger a few times to see if there might be still something lurking

@lidavidmlidavidm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for fixing this!

@alamb

Copy link
Copy Markdown
ContributorAuthor

Thanks @alamb !

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@alamb@lidavidm