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

Check RowsAffected when applying DML events to get more accurate statistics #844

Merged
merged 5 commits into from
Jul 14, 2021

Conversation

ajm188
Copy link
Contributor

@ajm188 ajm188 commented May 10, 2020

Addresses #600.

When applying a DML event, check the RowsAffected on the Result struct. Since all DML event queries are point queries, this will only ever be 0 or 1. The applier then takes this value and multiplies by the rowsDelta of the event, resulting in a properly-signed, accurate row delta to use in the statistics.

If an error occurs here, log it, but do not surface this as an actual error .. simply assume the DML affected a row and move on. It will be inaccurate, but this is already the case.

In case this PR introduced Go code changes:

  • contributed code is using same conventions as original code
  • script/cibuild returns with no formatting errors, build errors or unit test errors.

…istics

Addresses github#600.

When applying a DML event, check the RowsAffected on the `Result`
struct. Since all DML event queries are point queries, this will only
ever be 0 or 1. The applier then takes this value and multiplies by
the `rowsDelta` of the event, resulting in a properly-signed, accurate
row delta to use in the statistics.

If an error occurs here, log it, but do not surface this as an
actual error .. simply assume the DML affected a row and move on. It
will be inaccurate, but this is already the case.
@timvaillancourt timvaillancourt added this to the v1.1.1 milestone Dec 24, 2020
@timvaillancourt timvaillancourt modified the milestones: v1.1.1, v1.2.0, v1.3.0 May 3, 2021
@timvaillancourt timvaillancourt modified the milestones: v1.2.0, v1.1.3 Jun 17, 2021
@timvaillancourt timvaillancourt self-requested a review July 4, 2021 21:04
@timvaillancourt timvaillancourt temporarily deployed to production/mysql_role=ghost_testing July 4, 2021 21:09 Inactive
@timvaillancourt timvaillancourt temporarily deployed to production/mysql_role=ghost_testing July 4, 2021 21:09 Inactive
@shlomi-noach
Copy link
Contributor

Noting that we've seen issues with exact-rowcount taking longer to run than the entire migration, and are looking to recommend disabling exact-rowcount.

go/logic/applier.go Outdated Show resolved Hide resolved
@timvaillancourt
Copy link
Collaborator

Merging. Thanks @ajm188!

@timvaillancourt timvaillancourt merged commit 6e1daf9 into github:master Jul 14, 2021
@dm-2 dm-2 mentioned this pull request Jul 7, 2022
@dm-2 dm-2 modified the milestones: v1.1.5, v1.1.6 Jul 7, 2022
dm-2 pushed a commit that referenced this pull request Jul 7, 2022
…istics (#844)

* Check RowsAffected when applying DML events to get more accurate statistics

Addresses #600.

When applying a DML event, check the RowsAffected on the `Result`
struct. Since all DML event queries are point queries, this will only
ever be 0 or 1. The applier then takes this value and multiplies by
the `rowsDelta` of the event, resulting in a properly-signed, accurate
row delta to use in the statistics.

If an error occurs here, log it, but do not surface this as an
actual error .. simply assume the DML affected a row and move on. It
will be inaccurate, but this is already the case.

* Fix import

* update wording to warning log message

Co-authored-by: Tim Vaillancourt <[email protected]>
dm-2 pushed a commit that referenced this pull request Jul 7, 2022
…istics (#844)

* Check RowsAffected when applying DML events to get more accurate statistics

Addresses #600.

When applying a DML event, check the RowsAffected on the `Result`
struct. Since all DML event queries are point queries, this will only
ever be 0 or 1. The applier then takes this value and multiplies by
the `rowsDelta` of the event, resulting in a properly-signed, accurate
row delta to use in the statistics.

If an error occurs here, log it, but do not surface this as an
actual error .. simply assume the DML affected a row and move on. It
will be inaccurate, but this is already the case.

* Fix import

* update wording to warning log message

Co-authored-by: Tim Vaillancourt <[email protected]>
Sign up for free to 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.

4 participants