Skip to content

Commit

Permalink
sql: pass stopper.ShouldQuiesce() to retryFunc for TemporaryObjectCle…
Browse files Browse the repository at this point in the history
…aner

Resolves #47047.

This fixes a bug where the temporary object cleaner can hang during
object shutdown by not obeying stopper.ShouldQuiesce(). I've run
`TestChartCatalogGen` 10 times and confirmed it all takes the same
amount of time.

Release note: None
  • Loading branch information
otan committed Apr 5, 2020
1 parent c347574 commit 6dd1562
Showing 1 changed file with 10 additions and 3 deletions.
13 changes: 10 additions & 3 deletions pkg/sql/temporary_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,12 +417,19 @@ func makeTemporaryObjectCleanerMetrics() *temporaryObjectCleanerMetrics {
}

// doTemporaryObjectCleanup performs the actual cleanup.
func (c *TemporaryObjectCleaner) doTemporaryObjectCleanup(ctx context.Context) error {
func (c *TemporaryObjectCleaner) doTemporaryObjectCleanup(
ctx context.Context, closerCh <-chan struct{},
) error {
// Wrap the retry functionality with the default arguments.
retryFunc := func(ctx context.Context, do func() error) error {
return retry.WithMaxAttempts(
ctx,
retry.Options{InitialBackoff: 1 * time.Second, MaxBackoff: 1 * time.Minute, Multiplier: 2},
retry.Options{
InitialBackoff: 1 * time.Second,
MaxBackoff: 1 * time.Minute,
Multiplier: 2,
Closer: closerCh,
},
5, // maxAttempts
func() error {
err := do()
Expand Down Expand Up @@ -547,7 +554,7 @@ func (c *TemporaryObjectCleaner) Start(ctx context.Context, stopper *stop.Stoppe

select {
case <-nextTickCh:
if err := c.doTemporaryObjectCleanup(ctx); err != nil {
if err := c.doTemporaryObjectCleanup(ctx, stopper.ShouldQuiesce()); err != nil {
log.Warningf(ctx, "failed to clean temp objects: %v", err)
}
case <-stopper.ShouldQuiesce():
Expand Down

0 comments on commit 6dd1562

Please sign in to comment.