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.

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

Fault tolerant execution for SqlServer connector #14730

Merged
merged 2 commits into from
Oct 25, 2022

Conversation

mwd410
Copy link
Contributor

@mwd410 mwd410 commented Oct 24, 2022

Description

This pr enables fault tolerant execution for the SqlServer connector. It builds on the work done in my previous PR, which is why these changes are so minimal. The main change is passing true to the SqlServerClient's super constructor's supportsRetries parameter.

Non-technical explanation

This enables FTE for SqlServer and adds fault tolerance tests for the same.

Release notes

( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
(X) Release notes are required, with the following suggested text:
Enables fault tolerant execution on SqlServer connector.

# Section
* Allow writes to SqlServer connector when fault tolerant execution is enabled (`retry-policy` is set to `TASK` or `QUERY`). (https://github.com/starburstdata/stargate/issues/3877)

@cla-bot cla-bot bot added the cla-signed label Oct 24, 2022
@mwd410 mwd410 force-pushed the mdeady-sqlserver-fte-3877 branch from 650dabd to 421df72 Compare October 24, 2022 16:33
@electrum
Copy link
Member

Update the documentation at docs/src/main/sphinx/admin/fault-tolerant-execution.rst to include this connector as part of the second commit

<configuration>
<threadCount>4</threadCount>
<includes>
<include>**/io/trino/faulttolerant/sqlserver/Test*.java</include>
Copy link
Member

Choose a reason for hiding this comment

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

update excludes section above:

<excludes>
                                <exclude>**/io/trino/faulttolerant/delta/Test*.java</exclude>
                                <exclude>**/io/trino/faulttolerant/hive/Test*.java</exclude>
                                <exclude>**/io/trino/faulttolerant/iceberg/Test*.java</exclude>
                                <exclude>**/io/trino/faulttolerant/mysql/Test*.java</exclude>
                                <exclude>**/io/trino/faulttolerant/postgresql/Test*.java</exclude>
                            </excludes>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@losipiuk
Copy link
Member

Update the documentation at docs/src/main/sphinx/admin/fault-tolerant-execution.rst to include this connector as part of the second commit

Yeah - also add mysql and postgres there. In a separate commit (first in this PR).

@losipiuk losipiuk self-requested a review October 25, 2022 13:29
Copy link
Member

@losipiuk losipiuk left a comment

Choose a reason for hiding this comment

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

LGTM modulo ignore

@losipiuk
Copy link
Member

Also see failing tests - you need to adjust dependencies in pom.

@mwd410 mwd410 force-pushed the mdeady-sqlserver-fte-3877 branch from 421df72 to d05f782 Compare October 25, 2022 14:58
@mwd410 mwd410 force-pushed the mdeady-sqlserver-fte-3877 branch from d05f782 to e625b93 Compare October 25, 2022 15:23
@github-actions github-actions bot added the docs label Oct 25, 2022
@losipiuk losipiuk merged commit fffe117 into trinodb:master Oct 25, 2022
@github-actions github-actions bot added this to the 401 milestone Oct 25, 2022
@mwd410 mwd410 deleted the mdeady-sqlserver-fte-3877 branch October 26, 2022 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants