Skip to content

Add checkpoint info to /stats API endpoint, add checkpoint info UI#6003

Draft
Karakatiza666 wants to merge 3 commits intomainfrom
checkpoint-ui
Draft

Add checkpoint info to /stats API endpoint, add checkpoint info UI#6003
Karakatiza666 wants to merge 3 commits intomainfrom
checkpoint-ui

Conversation

@Karakatiza666
Copy link
Copy Markdown
Contributor

@Karakatiza666 Karakatiza666 commented Apr 7, 2026

Testing:
Tested the UI using the mock API value simulator. Did not test the adapters and pipeline-manager changes

image image image image image

Copy link
Copy Markdown

@mythical-fred mythical-fred left a comment

Choose a reason for hiding this comment

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

Two design questions, see inline.


/// Converts epoch milliseconds to a `DateTime<Utc>`, falling back to
/// `Utc::now()` if the value is 0 or invalid.
fn epoch_ms_to_datetime(epoch_ms: i64) -> DateTime<Utc> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If epoch_ms is 0/invalid, returning Utc::now() will make the UI show a fresh timestamp even when we don’t actually know the start time. Consider returning an Option (and omitting the field) or a sentinel so the UI can show "unknown" instead of a misleading "just now".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The epoch_ms should not happen at runtime; updated the comments and the code to reflect that. Since this is a hard rule I opted for an explicit crash if this occurs, LMK if you think it's better to bubble it up as an error

let epoch_ms = self.checkpoint_started_epoch_ms();
let started_at = epoch_ms_to_datetime(epoch_ms);
CheckpointActivity::InProgress {
sequence_number: 0,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

sequence_number: 0 looks like a placeholder. If we can’t plumb the real seq here, consider making this field optional or dropping it from CheckpointActivity::InProgress for now to avoid exposing bogus data.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Dropped sequence_number because it is not available in all cases, and we don't have a business case to display it for an in-progress checkpoint anyway

@gz
Copy link
Copy Markdown
Contributor

gz commented Apr 7, 2026

please engage with anna for the UX

@ryzhyk
Copy link
Copy Markdown
Contributor

ryzhyk commented Apr 7, 2026

Some feedback on UI:

  • "2 checkpoints at 780MiB" -> "2 checkpoints, total size: 780MiB"
  • Checkpoint list: include the timestamp, including time zone (UTC), when the checkpoint was made with each checkpoint (can be extracted from ULID). This should be visible even in the collapsed state.
  • "Make checkpoint" - if we expose this, it shouldn't be hiding in the checkpoint list.

Signed-off-by: Karakatiza666 <bulakh.96@gmail.com>
…timestamp

Signed-off-by: Karakatiza666 <bulakh.96@gmail.com>
Signed-off-by: feldera-bot <feldera-bot@feldera.com>
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.

5 participants