apache / apache/pulsar

[improve] Make managed cursor persist-error metrics visible by default

Open
#26,488 0 comments 0 reactions 1 assignee Claimed by @lhotari View on GitHub
type/enhancement
Dominant language
Java
Stars
15.3k
Forks
3.8k
Avg merge
1d 14h
Merged PRs (30d)
160

Description

### Search before reporting

- [X] I searched in the [issues](https://github.com/apache/pulsar/issues) and found nothing similar.

### Motivation

`brk_ml_cursor_persistLedgerErrors` / `brk_ml_cursor_persistLedgerSucceed` and their ZooKeeper counterparts are the only metrics that reveal a cursor failing to persist its mark-delete position. They are behind `exposeManagedCursorMetricsInPrometheus`, which defaults to `false` (`ServiceConfiguration.java:3883` on `master` `9ba61bd95de`).

That default makes a whole class of failure undetectable in practice:

- a cursor whose ledger cannot be created or written keeps serving reads and produces no metric movement;
- most of the corresponding broker-side signals are DEBUG-level, so the logs do not fill the gap either;
- `pulsar_ml_AddEntryErrors` is frequently assumed to cover it, but `recordAddEntryError()` has exactly two callers, both in `OpAddEntry` — that is the topic **data** write path, not the cursor persist path. Cursor persists are counted only by the cursor metrics above.

The result is that "the cursor is not advancing" has no first-class signal on a default deployment, and operators reach for metrics that cannot answer the question.

### Solution

Any of, in decreasing order of preference:

1. Export the cursor persist-error counters unconditionally, independent of `exposeManagedCursorMetricsInPrometheus` — they are per-cursor counters with low cardinality relative to the full managed-cursor metric set that the flag was introduced to gate.
2. Change the default of `exposeManagedCursorMetricsInPrometheus` to `true`.
3. Keep the flag as-is but emit a WARN when a cursor persist fails, so the condition is at least visible in logs at default levels.

Clarifying the documentation of `pulsar_ml_AddEntryErrors` to state that it does not cover cursor persists would also help.

### Alternatives

Leaving it as-is and documenting that operators must enable the flag proactively. This does not help during an incident, since the flag is a broker restart away and the historical data does not exist.

### Anything else?

Related: #25528 made acknowledgements with receipts reflect persistence, which improves client-side visibility. This request is about broker-side visibility for cursors with no client waiting on them — the compaction cursor in particular.

### Are you willing to submit a PR?

- [X] I'm willing to submit a PR!

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.