Skip to content

rptest: fail fast on crashed kgo-verifier worker with cause#30973

Open
sjust-redpanda wants to merge 1 commit into
redpanda-data:devfrom
sjust-redpanda:sjust/core-16624
Open

rptest: fail fast on crashed kgo-verifier worker with cause#30973
sjust-redpanda wants to merge 1 commit into
redpanda-data:devfrom
sjust-redpanda:sjust/core-16624

Conversation

@sjust-redpanda

Copy link
Copy Markdown
Contributor

ClusterLinkingProgressVerifier.validate_progress waited out the full progress_timeout and raised an opaque "Workload stalled" whenever a worker stopped advancing — including when a kgo-verifier process had crashed outright (e.g. a franz-go produce-path panic), which can never recover. That conflated a client-side worker crash with a genuine shadow-link replication stall, and forced a needless progress_timeout wait before failing.

This PR checks each worker's StatusThread.errored every iteration and fails fast with a distinct, classified error. Since errored only tells us the worker became unreachable (process died) and not why, it tails the worker's log on the node to label the cause:

Failure Example log tail Label
Go panic (internal client bug, e.g. the franz-go produce-path panic) panic: runtime error: index out of range [-1] internal client crash (panic)
Produce failure from broker (util.Dieos.Exit) level=error msg="Produce failed: UNKNOWN_TOPIC_OR_PARTITION: ..." fatal error propagated through the protocol
Data-loss detection (log.Fatalf) level=fatal msg="Unexpected data loss detected: ..." fatal error propagated through the protocol

The reason is appended to the raised error (e.g. kgo-verifier worker crashed: producer (internal client crash (panic))) and the full tail is logged. After this, a plain "Workload stalled" reliably means a real replication stall rather than a crashed client.

Log reads are best-effort and fall back to "cause unknown" so classification never masks the underlying crash. A worker that surfaces a fatal error in-status while staying alive (e.g. --tolerate-failed-produce) is intentionally not treated as a crash.

Validated against the real kgo-verifier binary: the panic path matches a captured production crash log, and the produce-failure path was reproduced live (delete topic mid-produce → level=error … Produce failed: UNKNOWN_TOPIC_OR_PARTITIONos.Exit(1)).

Motivating context: CORE-16624, where the franz-go producer-crash and a real target-side replication stall both surfaced as the same "Workload stalled" assertion.

Backports Required

  • none - not a bug fix

Release Notes

  • none

ClusterLinkingProgressVerifier.validate_progress waited out the full
progress_timeout and raised an opaque "Workload stalled" whenever a
worker stopped advancing -- including when a kgo-verifier process had
crashed outright (e.g. a franz-go produce-path panic), which can never
recover. That conflated a client-side worker crash with a genuine
shadow-link replication stall and forced a needless progress_timeout
wait before failing.

Each worker's StatusThread.errored flips when its process dies and stops
answering status polls. Check it every iteration and fail fast with a
distinct, classified error. The class only tells us the worker became
unreachable, not why, so tail its log on the node to label the cause: a
Go "panic" => an unexpected internal client crash; a fatal/error line
logged before exit => a fatal error propagated through the Kafka
protocol. The reason is appended to the raised error and the full tail
is logged for diagnosis.

Example log tails per type:
  internal client crash (panic):
    panic: runtime error: index out of range [-1]
  fatal error propagated through the protocol (produce failure):
    level=error msg="Produce failed: UNKNOWN_TOPIC_OR_PARTITION: ..."
  fatal error propagated through the protocol (data loss):
    level=fatal msg="Unexpected data loss detected: ..."

Best-effort: log reads are tolerant of failure and fall back to
"cause unknown" so classification never masks the underlying crash. A
plain "Workload stalled" then reliably means a real replication stall
rather than a crashed client.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vbotbuildovich

Copy link
Copy Markdown
Collaborator

Retry command for Build#86548

please wait until all jobs are finished before running the slash command

/ci-repeat 1
skip-redpanda-build
skip-units
skip-rebase
tests/rptest/tests/random_node_operations_smoke_test.py::RedpandaNodeOperationsSmokeTest.test_node_ops_smoke_test@{"cloud_storage_type":1,"mixed_versions":false}

@vbotbuildovich

Copy link
Copy Markdown
Collaborator

CI test results

test results on build#86548
test_status test_class test_method test_arguments test_kind job_url passed reason test_history
FLAKY(FAIL) RedpandaNodeOperationsSmokeTest test_node_ops_smoke_test {"cloud_storage_type": 1, "mixed_versions": false} integration https://buildkite.com/redpanda/redpanda/builds/86548#019f1a7e-8cc6-46bf-a407-07c0c727d262 5/11 Test FAILS after retries.Significant increase in flaky rate(baseline=0.0152, p0=0.0000, reject_threshold=0.0100) https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=RedpandaNodeOperationsSmokeTest&test_method=test_node_ops_smoke_test

@nvartolomei nvartolomei left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be done much better.

  1. Don't reach into services internals from the test (i.e. accessing 'private' fields svc._status_thread)
  2. Don't inspect service internals (like logs) from external services.

You can call svc.status_thread.raise_on_error() I reckon. On crash it will raise. Crash exceptions look even better starting with #30943.

I have avoided parsing logs and including them in exception messages as I was afraid the error messages are used as-is with some exception for grouping ci failures. If that's not the case (message used verbatim in signature) and you want logs included in the failure message then I suggest extending code at https://github.com/redpanda-data/redpanda/pull/30943/changes#diff-205d055201eb5b8c927e7b298891fa345b803a7f00663ce444744ac2e5948e2eR290-R292 to augment it with log contents.

Also, all tests will benefit from this.

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.

3 participants