Skip to content

terms: skip injected code for off actions - #93

Open
1fanwang wants to merge 1 commit into
pingcap:masterfrom
1fanwang:1fannnw/skip-off-failpoint
Open

1fanwang wants to merge 1 commit into
pingcap:masterfrom
1fanwang:1fannnw/skip-off-failpoint

Conversation

@1fanwang

Copy link
Copy Markdown

What problem does this PR solve?

Setting a failpoint to off still runs the injected code. This can inject a failure even after the caller has switched the action off.

Fixes #86.

What is changed and how it works?

The off action returns the existing non-triggering error instead of a successful evaluation. Generated injection guards then skip that evaluation. Counted off terms can still advance to a later action.

Check List

  • Unit tests cover off actions, term chains, exhausted counts and enabled controls.
  • The README states the evaluation result for an off action.
  • The manual check below uses the real source rewriter and executes its output.

Testing Done

The same rewritten program prints term=off injected=true before the fix and term=off injected=false afterward. The enabled control remains true.

I ran the probe from an unchanged checkout of 55ac33a and from this branch.

Save this program as /tmp/failpoint-off-probe-XyxNHL/main.go:

package main

import (
	"fmt"

	"github.com/pingcap/failpoint"
)

func main() {
	for _, term := range []string{"return", "off"} {
		if err := failpoint.Enable("main/off-repro", term); err != nil {
			panic(err)
		}
		injected := false
		failpoint.Inject("off-repro", func() {
			injected = true
		})
		fmt.Printf("term=%s injected=%t\n", term, injected)
	}
}

Rewrite it once from the baseline checkout:

GOMAXPROCS=2 go run ./failpoint-ctl enable /tmp/failpoint-off-probe-XyxNHL
Scenario Command, run from each checkout Observed result
Before GOMAXPROCS=2 go run /tmp/failpoint-off-probe-XyxNHL/*.go Both actions execute the injection.
After GOMAXPROCS=2 go run /tmp/failpoint-off-probe-XyxNHL/*.go Only the return action executes it.
Raw logs

Before:

term=return injected=true
term=off injected=true

After:

term=return injected=true
term=off injected=false

The separate rewriter test TestRewriteInjectCall/case-0 fails on both the unchanged baseline and this branch: its expected output still includes an evaluation guard that the rewriter no longer emits. This change leaves that pre-existing failure untouched.

Return ErrNotAllowed for off so generated Eval guards skip the injection.

Fixes pingcap#86

Signed-off-by: 1fanwang <1fannnw@gmail.com>
@pingcap-cla-assistant

pingcap-cla-assistant Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

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.

expect Enable() inTerm:'off' does not trigger failpoint code, but it's not as expect

1 participant