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

Table GC: disable binary logging on best effort basis #7588

Merged

Conversation

shlomi-noach
Copy link
Contributor

Description

By way of optimization, our table lifecycle mechanism only purges rows on the primary tablet. It does so by disabling binary logging. Here's the explanation that is in the code:

	// Disable binary logging, re-enable afterwards
	// The idea is that DROP TABLE can be expensive, on the primary, if the table is not empty.
	// However, on replica the price is not as high. Therefore, we only purge the rows on the primary.
	// This saves a lot of load from the replication stream, avoiding excessive lags. It also
	// avoids excessive IO on the replicas.
	// (note that the user may skip the PURGE step if they want, but the step is on by default)

With this PR, we disable binary logging on best-effort basis. Here's the explanation that is in the PR's code:

	// However, disabling SQL_LOG_BIN requires SUPER privileges, and we don;t know that we have that.
	// e.g. on Amazon Aurora we do not have SUPER and can disable log bin.
	// We therefore disable log bin on best-effort base. Te logic is still fine and sound if binary logging
	// is left enabled. We just lose some optimization.

And so, running on e.g. Amazon Aurora, the lifecycle mechanism will not disable binary logging (it will try, but fail to, and silently proceed), and purge rows with binlogs enabled. Performance loss, but logic is sound.

Related Issue(s)

Checklist

  • Should this PR be backported?
  • Tests were added or are not required
  • Documentation was added or is not required

Impacted Areas in Vitess

Components that this PR will affect:

  • Query Serving
  • VReplication
  • Cluster Management
  • Build/CI
  • VTAdmin

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@shlomi-noach shlomi-noach changed the title Table GC: disable inary logging on best effort basis Table GC: disable binary logging on best effort basis Mar 3, 2021
Signed-off-by: Jacques Grove <aquarapid@gmail.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

Please fix the typo and then merge.

go/vt/vttablet/tabletserver/gc/tablegc.go Outdated Show resolved Hide resolved
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@shlomi-noach shlomi-noach merged commit a51aeb9 into vitessio:master Mar 9, 2021
@shlomi-noach shlomi-noach deleted the throttler-sql-log-bin-best-effort branch March 9, 2021 09:29
@askdba askdba added this to the v10.0 milestone Mar 10, 2021
@aquarapid
Copy link
Contributor

Sorry for getting in here late; but I did modify the transition state times locally ; and performed a manual end-to-end verification. All good!

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