tests: add robustness test for issue 17780

[fuweid]

Closes #17780

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[fuweid]

ping @ahrtr @serathius @siyuanfoundation

[siyuanfoundation]

Could this be inline if it is not used by any other tests?

[fuweid]

You mean inline is about anonymous value during assignment?
etcdTraffic is unexported type and choiceWeight is alsog unexported type...
I think it's hard to make it inline if I understand it correctly.

[siyuanfoundation]

I see. How about just name it to EtcdPutDeleteHeavy?

[serathius]

+1 merging it with other traffic and incorporating it with exploratory testing.

[fuweid]

Hi @siyuanfoundation @serathius , I want to keep it with special name here because it's hard to reproduce it without special input or setting. If we put it into exploratory testing, existing case might have chance to reproduce it. But it's unknown for us. I was trying many combinations of traffic but it runs one day without luck. And then we introduce compact traffic, it also makes it hard to trigger issue 17780.

If you have better idea, please let me know. Thanks

[serathius]

I would prefer to avoid having a issue specific traffic, to prevent regressions in robustness we don't need 100% reproducability, about 2-5% is ok.

[serathius]

I would like to better understand why this is not aligned with exploratory tests. The goal is to incorporate regressions into exploratory testing, not just create a new scenario.

[serathius]

Suggested change

	// AllowBatchCompactBeforeSetFinishedCompactPanic is used to trigger
	// that compactBeforeSetFinishedCompact failpoint only if the current
	// revision number is higher than that batch limit.
	AllowBatchCompactBeforeSetFinishedCompactPanic Failpoint = goPanicFailpoint{
	failpoint: "compactBeforeSetFinishedCompact",
	trigger: triggerCompact{multiBatchCompaction: true},
	target: AnyMember,
	}
	CompactBeforeSetFinishedCompactPanic Failpoint = goPanicFailpoint{"compactBeforeSetFinishedCompact", triggerCompact{multiBatchCompaction: true}, AnyMember}

[fuweid]

Hi @serathius , CompactBeforeSetFinishedCompactPanic is already existing at

etcd/tests/robustness/failpoint/gofail.go

Line 47 in 5790774

CompactBeforeSetFinishedCompactPanic Failpoint = goPanicFailpoint{"compactBeforeSetFinishedCompact", triggerCompact{}, AnyMember}

Do you mean that I should replace existing one?

I add new one here because this issue has a requirement that the compactor has to delete tombstone in one batch and update UnsafeSetFinishedCompact value in next round. IMO, it's better to have two go-failpoints here.

[serathius]

We can have both, best to even duplicate all failpoints with triggerCompact to have option with and without multiBatchCompaction.

[serathius]

See #17680 for an example of how to add new repro. I adjusted couple of parameters.

[k8s-ci-robot]

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[k8s-ci-robot]

@fuweid: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-etcd-integration-2-cpu-amd64	69152bf	link	false	/test pull-etcd-integration-2-cpu-amd64
pull-etcd-unit-test-arm64	69152bf	link	true	/test pull-etcd-unit-test-arm64

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.