Couple tablet refresh interval and availability gate in the operator by mcrauwel · Pull Request #797 · planetscale/vitess-operator

mcrauwel · 2026-06-29T07:17:37Z

Summary

Fixes #790.

The operator marked a restarted tablet Available after a hardcoded 30s and proceeded to drain the next replica — but vtgate's default --tablet_refresh_interval is 60s, so most vtgates hadn't rediscovered the first tablet yet, producing "no healthy tablet available for 'tablet_type:REPLICA'" errors during rolling restarts.

The operator already controls both sides of this contract, so it should keep them consistent itself rather than asking the customer to discover and tune the relationship. This PR introduces a single knob, tabletRefreshInterval, from which the operator derives both:

vtgate's --tablet_refresh_interval flag (previously left to vtgate's own default), and
the VitessShard availability gate that paces rolling restarts, computed as interval × 1.5 (rounded up).

They can no longer drift. The default is 60s — matching vtgate's built-in default — which yields a 90s availability gate, so the original bug is fixed out of the box with no customer action.

spec:
  # optional; default 60s. Operator derives:
  #   vtgate --tablet_refresh_interval = 60s
  #   shard availability gate          = 90s (= 60 × 1.5)
  tabletRefreshInterval: 60s

tabletRefreshInterval stays optional, mainly so large clusters can trade a shorter refresh interval (faster rolling restarts) against more topology-server polling load.

Why this shape

Operator does the right thing. The fix is safe by default; the customer doesn't need to know about the vtgate/operator timing coupling.
One source of truth. A single value drives both settings, so they can't be configured inconsistently. Setting --tablet_refresh_interval directly via ExtraVitessFlags/gateway ExtraFlags is documented as unsupported because it bypasses the coupling.

Changes

API: tabletRefreshInterval *metav1.Duration on VitessClusterSpec, inherited cluster→cell (feeds vtgate) and cluster→keyspace→shard (feeds the gate). Default 60s via DefaultVitessCell/DefaultVitessShard.
Derivation: TabletAvailableSeconds(refreshInterval) = ⌈interval × 1.5⌉, used by the VitessShard controller.
vtgate: baseFlags() sets tablet_refresh_interval from the cell's value.
Generated: deepcopy, CRDs, API docs.
Tests: derivation helper; defaulting (shard + cell); propagation on all three hops; the vtgate flag; and an end-to-end integration test asserting both the shard field and the derived vtgate flag.

Note

Also fixes a latent bug along the way: the availability requeue used time.Duration(n) (nanoseconds) instead of n * time.Second.

🤖 Generated with Claude Code

tabletAvailableSeconds was hardcoded to 30s, but vtgate's default --tablet_refresh_interval is 1m. During rolling restarts the operator could mark a restarted tablet Available and drain the next replica before most vtgates had discovered the first one via topology polling, causing "no healthy tablet available for 'tablet_type:REPLICA'" errors. Expose TabletAvailableSeconds as an optional *int32 field on VitessClusterSpec, inherited down through VitessKeyspace to VitessShard like other cluster-wide settings (ExtraVitessFlags, UpdateStrategy). It defaults to 30 for backwards compatibility, so operators can raise it to exceed their vtgate tablet_refresh_interval without forking the operator. Also fix a latent bug in the availability requeue: it passed time.Duration(tabletAvailableSeconds), which is 30 nanoseconds rather than 30 seconds. It now correctly multiplies by time.Second. Adds tests for the defaulting and for propagation at both hops (cluster->keyspace and keyspace->shard). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Matthias Crauwels <matthias.crauwels@planetscale.com>

Set tabletAvailableSeconds on the VitessCluster in the integration fixture and assert it propagates through the live controller reconcile chain down to each VitessShard CR. This exercises the full path (including CRD validation accepting the new field) that the unit tests, which call newVitessKeyspace/newVitessShard directly, do not. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Matthias Crauwels <matthias.crauwels@planetscale.com>

mattlord · 2026-06-29T13:51:05Z

+<p>TabletAvailableSeconds is how long a tablet Pod must be consistently Ready
+before the operator considers it Available and proceeds to the next step
+of a rolling update (such as draining the next tablet). This creates a
+safety buffer that accounts for the time it takes for vtgates to discover
+that the tablet is Ready and update their routing tables. If a tablet is
+Ready but vtgates don&rsquo;t know it yet, then it isn&rsquo;t actually available for
+serving queries.</p>
+<p>This should be set to at least vtgate&rsquo;s &ndash;tablet_refresh_interval (default
+1m) plus a small buffer, otherwise the operator may drain the next tablet
+before vtgates have discovered the previously restarted one, causing
+&ldquo;no healthy tablet available&rdquo; errors during rolling restarts.</p>
+<p>Default: 30</p>


I wonder when and why a user would configure this? Should we always use the vitess cluster resource's vtgate tablet-refresh-interval value * 2? This feels like a potential footgun to me, when I would say that always using something greater than the defined tablet refresh interval is simply the correct thing to do...

The main use case is large clusters where the 60s default makes rolling restarts painfully slow — at 120s per step (now with the ×2 multiplier you suggested), a shard with 6 replicas takes 12 minutes to roll. Operators of big clusters may want to bring that down by tuning a shorter refresh interval, accepting the tradeoff of more topology-server polling load.

The footgun risk you're pointing at is real, but the operator owns both sides of the equation — the vtgate flag and the availability gate are always interval × 2, so even if someone sets tabletRefreshInterval: 10s, they can't accidentally drift the two apart the way they could with the old tabletAvailableSeconds knob. The only remaining risk is setting it so low that vtgates can't reliably refresh within one cycle — and at that point it's a documented knob with explicit "don't set this directly via ExtraFlags" guidance.

That said, if you feel the coupling guarantee is sufficient and the knob isn't worth the added API surface, I'm comfortable making it internal-only (operator always uses vtgate's built-in default × 2, no user-facing field). Happy to go either way — just wanted to make sure the intended use case was clear before removing it.

mattlord · 2026-06-29T13:52:14Z

+	// Ready but vtgates don't know it yet, then it isn't actually available for
+	// serving queries.
+	//
+	// This should be set to at least vtgate's --tablet_refresh_interval (default


We should use the dashed version of the flag everywhere. The underscore variants will be going away at some point (you can use either today as part of the transition from underscores to dashes for all flags in Vitess).

Replaces the customer-tuned tabletAvailableSeconds knob with a single tabletRefreshInterval, from which the operator derives BOTH sides of the contract so they can never drift: - vtgate's --tablet_refresh_interval is set from the value (was left to vtgate's own default before). - The VitessShard availability gate that paces rolling restarts is computed as interval * 1.5 (rounded up). The default is 60s, matching vtgate's built-in --tablet_refresh_interval default, which yields a 90s availability gate. This fixes the original bug (operator draining the next replica before vtgates rediscover the last one) out of the box, without the customer having to know about the coupling or tune two settings. tabletRefreshInterval is inherited cluster -> cell (feeds vtgate) and cluster -> keyspace -> shard (feeds the gate). It remains optional, mainly so large clusters can trade a shorter refresh (faster restarts) against more topo-server polling load. Tests: derivation helper, defaulting (shard + cell), propagation on all three hops, the vtgate flag, and an end-to-end integration assertion covering both the shard field and the derived vtgate flag. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Matthias Crauwels <matthias.crauwels@planetscale.com>

mattlord

I love this new shape! ❤️

In pkg/controller/vitessshard/reconcile_tablets.go for the tabletAvailableStatus, it seems like we should requeue for the remaining time until the pod can become Available, not for the full derived window every time. With the new default this is 90s, so if a reconcile happens after the pod has already been Ready for 89s, we can still wait another 90s before marking it Available. That can almost double each rolling restart step. Kubernetes handles this as Ready.LastTransitionTime + minReadySeconds - now; I think that we should do the same here, with a small floor since this podutils version uses a strict Before check.

In pkg/operator/vtgate/deployment.go for the baseFlags, we should emit the canonical dashed Vitess flag name, tablet-refresh-interval, instead of tablet_refresh_interval, and update the tests/docs to match. Vitess still normalizes underscores today, but use of underscores is deprecated and the support for it will go away in a future release.

Otherwise it LGTM! Thank you, @mcrauwel !

frouioui

As the api specs are changing you will need to do this before merging: #629 (comment)

frouioui · 2026-07-02T18:38:31Z

+	if vtc.Spec.TabletRefreshInterval == nil {
+		vtc.Spec.TabletRefreshInterval = &metav1.Duration{Duration: DefaultTabletRefreshInterval}
+	}


We should create a DefaultTabletRefreshInterval function and follow the same pattern we're doing for the other fields.

Done — renamed the constant to unexported defaultTabletRefreshInterval and introduced an exported DefaultTabletRefreshInterval(**metav1.Duration) following the same pattern as DefaultUpdateStrategy. Both DefaultVitessShard and DefaultVitessCell now delegate to it.

frouioui · 2026-07-02T18:41:37Z

+	if dst.Spec.TabletRefreshInterval == nil {
+		dst.Spec.TabletRefreshInterval = &metav1.Duration{Duration: DefaultTabletRefreshInterval}
+	}


Same comment as https://github.com/planetscale/vitess-operator/pull/797/changes#r3515378256

Done here too.

frouioui · 2026-07-02T18:46:15Z

+	// The operator owns --tablet_refresh_interval so it stays consistent with
+	// the tablet-availability gate the VitessShard controller derives from the
+	// same value. It's normally defaulted by DefaultVitessCell; guard nil in
+	// case defaulting hasn't run.
+	if spec.Cell.TabletRefreshInterval != nil {
+		flags["tablet_refresh_interval"] = spec.Cell.TabletRefreshInterval.Duration.String()
+	}


We should start using the dash-version of the flag.

Done — switched to tablet-refresh-interval (dashes) in both deployment.go and its test, and updated the flag name in the VitessClusterSpec comment.

- Rename DefaultTabletRefreshInterval constant to unexported defaultTabletRefreshInterval and introduce an exported DefaultTabletRefreshInterval(*metav1.Duration) function following the same DefaultX pattern used for other nillable fields in the package (DefaultUpdateStrategy, etc.). Call sites in DefaultVitessShard and DefaultVitessCell are updated to use the new function. - Requeue for the remaining window rather than the full window when a tablet Pod is Ready but not yet Available. If a pod has been Ready for 119s of a 120s window it now requeues in 1s instead of 120s, halving the worst-case rolling restart step time. - Change the availability gate multiplier from 1.5x to 2x (interval x 2). A factor of 2 guarantees every vtgate has had at least one full refresh cycle to rediscover the tablet, and keeps the math simple. - Use the dashed flag name --tablet-refresh-interval everywhere; the underscore variant is deprecated in Vitess and will be removed. - Regenerate CRDs, API docs, and test operator YAMLs (controller-gen v0.20.1, already pinned in go.mod). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Matthias Crauwels <matthias.crauwels@planetscale.com>

mcrauwel · 2026-07-06T12:21:58Z

Re frouioui's operator.yaml comment: done — regenerated CRDs via make generate, built via kubectl kustomize ./deploy, and spliced the updated schemas into both test operator YAMLs while preserving their test-specific Deployment sections.

The auto-generated api.md/index.html had two entries with the underscore variant of the flag because VitessCellSpec.TabletRefreshInterval still referenced --tablet_refresh_interval. Update the source comment to match the dashed form used everywhere else, and regenerate docs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Matthias Crauwels <matthias.crauwels@planetscale.com>

mattlord

In test/integration/vitesscluster/vitesscluster_test.go:219-230 It seems like this test still expects the old underscore flag spelling, but pkg/operator/vtgate/deployment.go now emits --tablet-refresh-interval=40s. I think we should update the assertion and nearby comment to the dashed spelling, otherwise this integration test should fail even though the production code was updated correctly.

In pkg/operator/vtgate/deployment.go:234-240 I think we should enforce the new “operator owns tablet-refresh-interval” contract rather than only documenting it. baseFlags() sets tablet-refresh-interval from spec.Cell.TabletRefreshInterval, but then ExtraVitessFlags / gateway ExtraFlags are applied afterward and can still override tablet-refresh-interval, or add the underscore alias as a second normalized spelling. That seems like it can recreate the exact drift this PR is trying to remove. I think we should ignore or reject both tablet-refresh-interval and tablet_refresh_interval from vtgate extra flags, and add a small test for both spellings.

This is looking good, @mcrauwel ! ❤️

- Update the integration test to assert the dashed --tablet-refresh-interval=40s spelling, matching what deployment.go now emits. - Extract the vtgate extra-flags overlay into applyExtraFlags() and ignore tablet-refresh-interval / tablet_refresh_interval from user-provided flags, so ExtraVitessFlags and gateway ExtraFlags (merged into one map by the VitessCell controller) can no longer override the operator-derived value or re-add the underscore alias and recreate the drift this PR removes. - Add TestApplyExtraFlagsIgnoresTabletRefreshInterval covering both spellings. - Update the TabletRefreshInterval field doc from "do NOT set via extra flags" to "the operator ignores it there" and regenerate docs/api.md / docs/api/index.html. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matthias Crauwels <matthias.crauwels@planetscale.com>

mcrauwel · 2026-07-07T10:34:42Z

Thanks @mattlord — both addressed in 0fc55ba:

Integration test spelling: updated the assertion and comment in vitesscluster_test.go to the dashed --tablet-refresh-interval=40s.
Enforcing the contract: extracted the vtgate extra-flags overlay into applyExtraFlags(), which now ignores tablet-refresh-interval and tablet_refresh_interval (leading dashes tolerated, as before) from user-provided flags. Cluster-level ExtraVitessFlags and gateway ExtraFlags are merged into a single map by the VitessCell controller before reaching this point, so the one guard covers both paths. Added TestApplyExtraFlagsIgnoresTabletRefreshInterval covering both spellings (plus a dashed-prefix variant), asserting the operator-derived value survives, no underscore alias sneaks in as a second flag, and unrelated extra flags still apply. Also updated the TabletRefreshInterval API doc from "do NOT set this via extra flags" to "the operator ignores it there", and regenerated the docs.

Unit and integration suites pass locally.

mcrauwel force-pushed the mcrauwel/configurable-tablet-available-seconds branch from e2316aa to e6499ee Compare June 29, 2026 07:18

mattlord self-requested a review June 29, 2026 13:49

mattlord reviewed Jun 29, 2026

View reviewed changes

mcrauwel changed the title ~~Make tabletAvailableSeconds configurable (#790)~~ Couple tablet refresh interval and availability gate in the operator (#790) Jun 30, 2026

mattlord requested a review from frouioui July 2, 2026 16:56

mattlord reviewed Jul 2, 2026

View reviewed changes

frouioui reviewed Jul 2, 2026

View reviewed changes

mattlord changed the title ~~Couple tablet refresh interval and availability gate in the operator (#790)~~ Couple tablet refresh interval and availability gate in the operator Jul 6, 2026

mattlord reviewed Jul 6, 2026

View reviewed changes

Uh oh!

Conversation

mcrauwel commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why this shape

Changes

Note

Uh oh!

mattlord Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattlord left a comment

Choose a reason for hiding this comment

Uh oh!

frouioui left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcrauwel commented Jul 6, 2026

Uh oh!

mattlord left a comment

Choose a reason for hiding this comment

Uh oh!

mcrauwel commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mcrauwel commented Jun 29, 2026 •

edited

Loading

mattlord Jun 29, 2026 •

edited

Loading