Skip to content

schemachange: refactor GC job code#55737

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
spaskob:sc-simplify-gc
Oct 20, 2020
Merged

schemachange: refactor GC job code#55737
craig[bot] merged 1 commit intocockroachdb:masterfrom
spaskob:sc-simplify-gc

Conversation

@spaskob
Copy link
Copy Markdown
Contributor

@spaskob spaskob commented Oct 20, 2020

To implement #48775 I will be modifying the schema GC code to
add support for destroying tenants data. I noticed that the
current code has redundant repetitions which makes it hard to
understand and modify.

The logic for computing new deadline and refreshing tables is
being repeated in 3 code blocks - I have merged them together.

Release note: none.

@spaskob spaskob requested review from pbardea and thoszhang October 20, 2020 12:27
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Contributor

@pbardea pbardea left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @lucy-zhang, @pbardea, and @spaskob)


pkg/sql/gcjob/gc_job.go, line 153 at r1 (raw file):

				persistProgress(ctx, execCfg, r.jobID, progress)
			} else {
				// An element has expired but no deletion work has been done.

Perhaps we can add logging here for now? I think the linter might complain about an empty branch anyway.

To implement cockroachdb#48775 I will be modifying the schema GC code to
add support for destroying tenants data. I notice3d that the
current code has redundant repetitions which makes it hard to
understand and modify.

The logic for computing new deadline and refreshing tables is
being repeated in 3 code blocks - I have merged them together.

Release note: none.
Copy link
Copy Markdown
Contributor Author

@spaskob spaskob left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @lucy-zhang and @pbardea)


pkg/sql/gcjob/gc_job.go, line 153 at r1 (raw file):

Previously, pbardea (Paul Bardea) wrote…

Perhaps we can add logging here for now? I think the linter might complain about an empty branch anyway.

as discussed on slack we don't need to know if the performGC did any work

@spaskob
Copy link
Copy Markdown
Contributor Author

spaskob commented Oct 20, 2020

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Oct 20, 2020

Build succeeded:

@craig craig bot merged commit e16811f into cockroachdb:master Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants