sessionctx, config: gate FK shared-lock sysvar on next-gen
Project: pingcap/tidb | Language: Go | Concepts: configuration-management, api-design, testing-strategy | Difficulty: mid
Link: https://github.com/pingcap/tidb/pull/70328
The problem
The system variable tidb_foreign_key_check_in_shared_lock enables a foreign-key shared-lock feature that is still experimental and not generally available (GA), especially on next-gen TiKV deployments. Without extra safeguards, any user with permission to set this sysvar could enable the feature in production, potentially causing instability or unsupported behavior.
Why it's non-trivial
A naive fix might just hide or disable the sysvar, but that would break existing deployments that already have the setting enabled or need to maintain backward compatibility. The solution must:
- Prevent new enablement of the feature on next-gen TiKV unless explicitly allowed.
- Not break existing clusters that already have the feature enabled.
- Allow non-nextgen deployments to continue using the sysvar as before.
- Be testable and not disrupt test infrastructure.
The approach
1. Add a config gate to the experimental section
A new field is added to the experimental config to control whether SQL users can enable the shared-lock sysvar on next-gen TiKV:
type Experimental struct {
// Whether enable creating expression index.
AllowsExpressionIndex bool `toml:"allow-expression-index" json:"allow-expression-index"`
+ // Whether SQL users can enable tidb_foreign_key_check_in_shared_lock on next-gen TiKV.
+ AllowEnableForeignKeyCheckInSharedLock bool `toml:"allow-enable-foreign-key-check-in-shared-lock" json:"allow-enable-foreign-key-check-in-shared-lock"`
// Whether enable charset feature.
EnableNewCharset bool `toml:"enable-new-charset" json:"-"`
}
What this does: Introduces a config flag that defaults to false for next-gen deployments, gating the sysvar.
2. Document and test the config field
The example config and config tests are updated to include the new field and verify loading:
+# allow SQL users to enable tidb_foreign_key_check_in_shared_lock on next-gen TiKV.
+allow-enable-foreign-key-check-in-shared-lock = false
What this does: Makes the config option visible and testable in documentation and test cases.
3. Gate sysvar enablement in code
The sysvar definition is updated to check the config gate before allowing the sysvar to be set to ON:
+func allowSetForeignKeyCheckInSharedLock() bool {
+ if !kerneltype.IsNextGen() {
+ return true
+ }
+ return config.GetGlobalConfig().Experimental.AllowEnableForeignKeyCheckInSharedLock
+}
...
+ {Scope: vardef.ScopeGlobal | vardef.ScopeSession, Name: vardef.TiDBForeignKeyCheckInSharedLock, Value: BoolToOnOff(vardef.DefTiDBForeignKeyCheckInSharedLock), Type: vardef.TypeBool, Validation: func(_ *SessionVars, normalizedValue string, originalValue string, _ vardef.ScopeFlag) (string, error) {
+ if TiDBOptOn(normalizedValue) && !allowSetForeignKeyCheckInSharedLock() {
+ return normalizedValue, ErrWrongValueForVar.GenWithStackByArgs(vardef.TiDBForeignKeyCheckInSharedLock, originalValue)
+ }
+ return normalizedValue, nil
+ }, SetSession: func(s *SessionVars, val string) error {
s.ForeignKeyCheckInSharedLock = TiDBOptOn(val)
return nil
+ }, GetSession: func(s *SessionVars) (string, error) {
+ return getForeignKeyCheckInSharedLockSession(s)
+ }, GetGlobal: func(_ context.Context, s *SessionVars) (string, error) {
+ if s.GlobalVarsAccessor == nil {
+ return BoolToOnOff(vardef.DefTiDBForeignKeyCheckInSharedLock), nil
+ }
+ return s.GlobalVarsAccessor.GetGlobalSysVar(vardef.TiDBForeignKeyCheckInSharedLock)
}},
What this does: Only allows the sysvar to be set to ON if the config gate is enabled, and only on next-gen TiKV.
4. Comprehensive testing
A new test is added to verify the gating logic in all scenarios: default, enabled, and fallback. It checks both session and global sysvar behavior.
+func TestTiDBForeignKeyCheckInSharedLockGate(t *testing.T) {
+ // ... setup ...
+ if !kerneltype.IsNextGen() {
+ // Should always allow
+ }
+ // Should reject enabling when gate is off
+ // Should allow when gate is on
+ // Should preserve behavior for already-set global values
+ // ...
+}
What this does: Ensures the gate works as intended and does not regress on existing behavior.
5. Enable the gate for integration tests
Integration tests that require the feature explicitly enable the config gate for their duration:
+func allowForeignKeyCheckInSharedLockForTest(t *testing.T) {
+ t.Helper()
+ restore := config.RestoreFunc()
+ t.Cleanup(restore)
+ config.UpdateGlobal(func(conf *config.Config) {
+ conf.Experimental.AllowEnableForeignKeyCheckInSharedLock = true
+ })
+}
...
+ allowForeignKeyCheckInSharedLockForTest(t)
What this does: Ensures tests for the shared-lock feature are not broken by the new gate.
From the review thread
No notable review discussion on this PR.
Takeaways
- Use configuration gates for experimental or non-GA features to prevent accidental enablement in production.
- Preserve backward compatibility by allowing already-enabled settings to persist across upgrades.
- Thoroughly test configuration gates, including edge cases (default, enabled, fallback, and persisted values).
- Integration tests for gated features should explicitly enable the gate and restore config after.