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.

Sign up for GitHub

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

Jump to bottom

Properly adhere to DbType#supportsParallelWrites in SqlLedger #2902

Merged
merged 2 commits into from
Sep 16, 2019
Merged

Properly adhere to DbType#supportsParallelWrites in SqlLedger #2902

merged 2 commits into from
Sep 16, 2019

Conversation

gerolf-da
Copy link
Contributor

@gerolf-da gerolf-da commented Sep 13, 2019 •
edited
Loading

Since we still have issues with parallel writes when using H2,
we should properly only use a single DB connection and executor thread
for that.

Because we didn't do that before for Sandbox, tests like PackageManagement
appeared to be flaky due to racy inserts in H2.

Pull Request Checklist

  • Read and understand the contribution guidelines
  • Include appropriate tests
  • Set a descriptive title and thorough description
  • Add a reference to the issue this PR will solve, if appropriate
  • Add a line to the release notes, if appropriate
  • Normal production system change, include purpose of change in description

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

@gerolf-da
Properly adhere to DbType#supportsParallelWrites in SqlLedger
0a36d9d 
Since we still have issues with parallel writes when using H2,
we should properly only use a single DB connection and executor thread
for that.

Because we didn't do that before for Sandbox, tests like PackageManagement
appeared to be flaky due to racy inserts in H2.
@gerolf-da gerolf-da added the component/ledger Sandbox and Ledger API label Sep 13, 2019
oliverse-da
oliverse-da approved these changes Sep 13, 2019
View reviewed changes
Copy link
Contributor

@oliverse-da oliverse-da left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I would also mention that we are no longer doubling the batchSize for ledger_entries inserts relative to the number of connections.

@gerolf-da gerolf-da merged commit ad1b885 into master Sep 16, 2019
@gerolf-da gerolf-da deleted the workaround-for-h2-flakiness branch September 16, 2019 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliverse-da oliverse-da oliverse-da approved these changes

@stefanobaghino-da stefanobaghino-da stefanobaghino-da approved these changes

@rautenrieth-da rautenrieth-da Awaiting requested review from rautenrieth-da

Assignees
No one assigned
Labels
component/ledger Sandbox and Ledger API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gerolf-da @stefanobaghino-da @oliverse-da