Skip to content

Conversation

@longvu-db
Copy link
Contributor

@longvu-db longvu-db commented Aug 18, 2024

Which Delta project/connector is this regarding?

  • Spark
  • Standalone
  • Flink
  • Kernel
  • Other (fill in here)

Description

For local E2E Delta Connect testing, we also designed an util class to start a local server in a different process similar to SparkConnect.

We noticed that the server takes a random amount of seconds to start up, and back then we received the error INVALID_HANDLE.SESSION_NOT_FOUND] The handle 746e6c86-9fa9-4b08-9572-388c20eaed47 is invalid. Session not found. SQLSTATE: HY000", so what we did is to add a 10s Thread.sleep before starting the client.

This is not robust, so we are removing the Thread.sleep. This should work because:

  1. The SparkSession's builder here already uses the default Configuration of the SparkConnectClient which includes a default retry policy.
  2. Spark patches the error INVALID_HANDLE.SESSION_NOT_FOUND in this PR at some point, so we should be able to retry even if encountering this error.

How was this patch tested?

Existing UTs.

Ran build/sbt tests locally at least 20 times.

Does this PR introduce any user-facing changes?

No.

@longvu-db longvu-db changed the title [Spark] Try removing Thead.sleep [Spark] Replacing Thead.sleep with SparkConnectClient's Retry Policy Aug 19, 2024
@longvu-db longvu-db changed the title [Spark] Replacing Thead.sleep with SparkConnectClient's Retry Policy [Spark] Replacing Thead.sleep to use Retry Policy instead in DeltaConnect's RemoteSparkSession Aug 19, 2024
@longvu-db longvu-db changed the title [Spark] Replacing Thead.sleep to use Retry Policy instead in DeltaConnect's RemoteSparkSession [Spark] Remove waiting for a fixed time for the Delta Connect Server to get ready in Delta Connect testing Aug 19, 2024
@longvu-db longvu-db changed the title [Spark] Remove waiting for a fixed time for the Delta Connect Server to get ready in Delta Connect testing [Spark] Remove waiting for a fixed time for Delta Connect Server to be available in Delta Connect testing Aug 19, 2024
Copy link
Collaborator

@tomvanbussel tomvanbussel left a comment

Choose a reason for hiding this comment

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

Not sure why these changes are necessary. The retry policy is already the default. Wouldn't just removing the sleep be enough?

@vkorukanti vkorukanti merged commit 56d057c into delta-io:master Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants