Add retry to test connections #23757

johnjcasey · 2022-10-20T17:00:38Z

fixes #22299
Please add a meaningful description for your change here

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Choose reviewer(s) and mention them in a comment (R: @username).
Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

…it parallel calls to datasource.getConnection()

johnjcasey · 2022-10-20T17:24:32Z

run beam_performanceTests_jdbc

johnjcasey · 2022-10-20T17:28:05Z

Run Java JdbcIO Performance Test

johnjcasey · 2022-10-20T18:39:56Z

Run Java JdbcIO Performance Test

johnjcasey · 2022-10-21T14:30:28Z

Run Java JdbcIO Performance Test

johnjcasey · 2022-10-21T17:02:46Z

Run Java JdbcIO Performance Test

johnjcasey · 2022-10-21T18:52:38Z

Run Java JdbcIO Performance Test

Put a retry into the DatabaseTestHelper

johnjcasey · 2022-10-24T16:26:51Z

Run Java JdbcIO Performance Test

johnjcasey · 2022-10-24T17:33:43Z

Run Java JdbcIO Performance Test

johnjcasey · 2022-10-25T13:04:41Z

Run Java JdbcIO Performance Test

johnjcasey · 2022-10-25T16:02:01Z

Run Java JdbcIO Performance Test

johnjcasey · 2022-10-26T14:54:13Z

r: @Abacn

github-actions · 2022-10-26T15:55:55Z

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @robertwb for label java.
R: @chamikaramj for label io.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

github-actions · 2022-10-26T15:59:59Z

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control

Abacn · 2022-10-26T16:44:11Z

I see,the cause is that the test did not retry create table. Can we utilize the backoff pattern to here?

beam/sdks/java/io/jdbc/src/main/java/org/apache/beam/sdk/io/jdbc/JdbcIO.java

Line 2345 in 840d66a

retryBackOff =

codecov · 2022-10-26T17:59:51Z

Codecov Report

Merging #23757 (840d66a) into master (1179fdc) will decrease coverage by 0.05%.
The diff coverage is 32.97%.

❗ Current head 840d66a differs from pull request most recent head 802601d. Consider uploading reports for the commit 802601d to get more accurate results

@@            Coverage Diff             @@
##           master   #23757      +/-   ##
==========================================
- Coverage   73.24%   73.19%   -0.06%     
==========================================
  Files         719      720       +1     
  Lines       95955    96069     +114     
==========================================
+ Hits        70279    70313      +34     
- Misses      24364    24444      +80     
  Partials     1312     1312

Flag	Coverage Δ
go	`50.69% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...pkg/beam/core/runtime/xlangx/expansionx/process.go	`61.40% <ø> (ø)`
...dks/python/apache_beam/options/pipeline_options.py	`94.45% <ø> (+0.08%)`	⬆️
...on/apache_beam/runners/dataflow/dataflow_runner.py	`81.65% <0.00%> (+0.76%)`	⬆️
...on/apache_beam/runners/portability/spark_runner.py	`67.34% <0.00%> (ø)`
...m/runners/portability/spark_uber_jar_job_server.py	`84.32% <0.00%> (ø)`
...eam/testing/benchmarks/nexmark/nexmark_launcher.py	`0.00% <0.00%> (ø)`
...pache_beam/typehints/pytorch_type_compatibility.py	`0.00% <0.00%> (ø)`
...python/apache_beam/io/gcp/bigquery_schema_tools.py	`79.24% <71.42%> (-4.85%)`	⬇️
sdks/python/apache_beam/io/gcp/gcsio.py	`92.12% <85.71%> (-0.14%)`	⬇️
sdks/python/apache_beam/ml/gcp/visionml.py	`92.59% <90.00%> (ø)`
... and 32 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Abacn · 2022-10-27T16:06:01Z

Run Java JdbcIO Performance Test

Abacn · 2022-10-27T17:05:03Z

Seems like org.postgresql.util.PSQLException is not caught by the catch (SQLException) https://ci-beam.apache.org/job/beam_PerformanceTests_JDBC/7101/console however PSQLException extends SQLException. Not sure why

Abacn · 2022-10-28T13:55:52Z

The try-with-resource statement does not catch the exception thrown in the parenthesis, I think. Replace to the simple try { getConnection } would work.

johnjcasey · 2022-10-28T14:50:49Z

it appears that it should catch the exception: https://stackoverflow.com/questions/15895227/try-with-resources-must-i-throw-or-catch-the-close-methods-exceptions

Abacn · 2022-10-28T14:58:25Z

sdks/java/io/common/src/test/java/org/apache/beam/sdk/io/common/DatabaseTestHelper.java

        try (Statement statement = connection.createStatement()) {
          statement.execute(String.format("create table %s (%s)", tableName, fieldsList));
          return;
        }
      } catch (SQLException e) {
        exception = e;
+      } finally {
+        if (connection!=null){
+          connection.close();


Probably it is closing the connection caused issue. dataSource owns a connection pool and manage the connections. Closing it pre-maturely may have caused racing conditions.

it looks like it is happening in .getConnection based on the stacktrace

johnjcasey · 2022-10-28T18:13:57Z

Run Java Precommit

johnjcasey · 2022-11-01T15:45:00Z

Run Java PreCommit

johnjcasey · 2022-11-22T16:55:54Z

@Abacn can you take another look at this?

Abacn

LGTM

Move connection setup logic for JDBCIO WriteFn to @startBundle to lim…

b250823

…it parallel calls to datasource.getConnection()

github-actions bot added io java jdbc labels Oct 20, 2022

johnjcasey changed the title ~~Move connection setup logic for JDBCIO WriteFn to @startBundle to lim…~~ Move connection setup logic for JDBCIO WriteFn to @startBundle to limit parallel calls to datasource.getConnection() Oct 21, 2022

move connection setup logic back to processElement.

3eb06bd

Put a retry into the DatabaseTestHelper

github-actions bot removed the jdbc label Oct 24, 2022

run spotless

d0c38cb

johnjcasey marked this pull request as ready for review October 26, 2022 14:54

johnjcasey changed the title ~~Move connection setup logic for JDBCIO WriteFn to @startBundle to limit parallel calls to datasource.getConnection()~~ Add retry to test connections Oct 26, 2022

github-actions bot added the Next Action: Reviewers label Oct 26, 2022

use fluent backoff instead of manual implementation

d31b84a

refactor to manual resource management

d7526fa

Abacn reviewed Oct 28, 2022

View reviewed changes

run spotless

802601d

johnjcasey self-assigned this Oct 28, 2022

Abacn approved these changes Nov 22, 2022

View reviewed changes

johnjcasey merged commit 67e2008 into apache:master Nov 22, 2022

Abacn mentioned this pull request Dec 29, 2022

JdbcIOIT flaky when run using io-it-suite-local #18432

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add retry to test connections #23757

Add retry to test connections #23757

johnjcasey commented Oct 20, 2022 •

edited

Loading

johnjcasey commented Oct 20, 2022

johnjcasey commented Oct 20, 2022

johnjcasey commented Oct 20, 2022

johnjcasey commented Oct 21, 2022

johnjcasey commented Oct 21, 2022

johnjcasey commented Oct 21, 2022

johnjcasey commented Oct 24, 2022

johnjcasey commented Oct 24, 2022

johnjcasey commented Oct 25, 2022

johnjcasey commented Oct 25, 2022

johnjcasey commented Oct 26, 2022

github-actions bot commented Oct 26, 2022

github-actions bot commented Oct 26, 2022

Abacn commented Oct 26, 2022 •

edited

Loading

codecov bot commented Oct 26, 2022 •

edited

Loading

Abacn commented Oct 27, 2022

Abacn commented Oct 27, 2022

Abacn commented Oct 28, 2022

johnjcasey commented Oct 28, 2022

Abacn Oct 28, 2022 •

edited

Loading

johnjcasey Oct 28, 2022

johnjcasey commented Oct 28, 2022

johnjcasey commented Nov 1, 2022

johnjcasey commented Nov 22, 2022

Abacn left a comment

Add retry to test connections #23757

Add retry to test connections #23757

Conversation

johnjcasey commented Oct 20, 2022 • edited Loading

GitHub Actions Tests Status (on master branch)

johnjcasey commented Oct 20, 2022

johnjcasey commented Oct 20, 2022

johnjcasey commented Oct 20, 2022

johnjcasey commented Oct 21, 2022

johnjcasey commented Oct 21, 2022

johnjcasey commented Oct 21, 2022

johnjcasey commented Oct 24, 2022

johnjcasey commented Oct 24, 2022

johnjcasey commented Oct 25, 2022

johnjcasey commented Oct 25, 2022

johnjcasey commented Oct 26, 2022

github-actions bot commented Oct 26, 2022

github-actions bot commented Oct 26, 2022

Abacn commented Oct 26, 2022 • edited Loading

codecov bot commented Oct 26, 2022 • edited Loading

Codecov Report

Abacn commented Oct 27, 2022

Abacn commented Oct 27, 2022

Abacn commented Oct 28, 2022

johnjcasey commented Oct 28, 2022

Abacn Oct 28, 2022 • edited Loading

Choose a reason for hiding this comment

johnjcasey Oct 28, 2022

Choose a reason for hiding this comment

johnjcasey commented Oct 28, 2022

johnjcasey commented Nov 1, 2022

johnjcasey commented Nov 22, 2022

Abacn left a comment

Choose a reason for hiding this comment

johnjcasey commented Oct 20, 2022 •

edited

Loading

Abacn commented Oct 26, 2022 •

edited

Loading

codecov bot commented Oct 26, 2022 •

edited

Loading

Abacn Oct 28, 2022 •

edited

Loading