Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui, changefeeds: better status col in db console #81213

Merged

Conversation

dhartunian
Copy link
Collaborator

@dhartunian dhartunian commented May 12, 2022

Previously, when a job contained a highwater_timestamp column value
(present for changefeeds) the status column in DB console would
always show that value instead of the job status ("running", "paused",
etc.). This would cause confusion for operators because the SQL output
for job status always included both a status column and a separate
highwater_timestamp column.

This change moves the highwater_timestamp into a separate column and
always renders the status column with the "pill" component that shows
the current job status.

The highwater timestamp is also moved to the sidebar in the job details
page instead of replacing the status pill, for similar consistency.

Finally, the highwater timestamp now displays the nanosecond decimal
value by default and the human-readable formatted value in the tooltip.
This faciliates easier copy/paste behavior from the UI as the decimal is
more useful.

Screenshot 2022-05-12 at 16-09-19 Jobs Cockroach Console
Screenshot 2022-05-12 at 16-09-47 Details Job Cockroach Console

Resolves #80496

Release note (ui change): The job status page in the DB Console will now
show the status column for changefeed jobs and display the
highwater_timestamp value in a separate column. Thise more closely
matches the SQL output of SHOW changefeed JOBS. The highwater
timestamp now displays as the nanosecond system time value by default
with the human-readable value in the tooltip since the decimal value is
copy/pasted more often.

@dhartunian dhartunian requested review from a team and stevendanna and removed request for a team May 12, 2022 15:33
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@shermanCRL shermanCRL removed the request for review from stevendanna May 12, 2022 15:34
@shermanCRL
Copy link
Contributor

I wonder if we should display the integer version of the highwater timestamp (as opposed to the friendly date) by default? Thinking of the use case that someone may wish to grab a copy of it for use elsewhere.

@HonoreDB
Copy link
Contributor

HonoreDB commented May 12, 2022

I wonder if we should display the integer version of the highwater timestamp (as opposed to the friendly date) by default? Thinking of the use case that someone may wish to grab a copy of it for use elsewhere.

Both versions are definitely useful, the precise cursor for restarting feeds and the friendly date for troubleshooting. Do we have the UI pattern anywhere where we use the friendly date as alt text so you see it when you hover?

Edit: Oh, I see, you're doing the reverse. Yeah, I agree with @shermanCRL that it'd be useful to have the integer version as the default since it's easier to highlight and copy page text than a tooltip.

Copy link
Contributor

@HonoreDB HonoreDB left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @dhartunian)

@HonoreDB
Copy link
Contributor

Thanks for doing this! Code LGTM. CI failure looks like a js dependency vendoring issue?

@dhartunian dhartunian force-pushed the improved-changefeed-status-db-console branch from 3624df3 to c23031f Compare May 12, 2022 20:29
@dhartunian
Copy link
Collaborator Author

@shermanCRL @HonoreDB updated PR with new screenshots that use the nanosecond decimal value as primary display.

@dhartunian dhartunian requested a review from HonoreDB May 12, 2022 20:31
@dhartunian dhartunian force-pushed the improved-changefeed-status-db-console branch 3 times, most recently from 4ca97e1 to f1a77a4 Compare May 16, 2022 16:25
Previously, when a job contained a `highwater_timestamp` column value
(present for changefeeds) the status column in DB console would
*always* show that value instead of the job status ("running", "paused",
etc.). This would cause confusion for operators because the SQL output
for job status always included both a `status` column and a separate
`highwater_timestamp` column.

This change moves the `highwater_timestamp` into a separate column and
always renders the `status` column with the "pill" component that shows
the current job status.

The highwater timestamp is also moved to the sidebar in the job details
page instead of replacing the status pill, for similar consistency.

Finally, the highwater timestamp now displays the nanosecond decimal
value by default and the human-readable formatted value in the tooltip.
This faciliates easier copy/paste behavior from the UI as the decimal is
more useful.

Resolves cockroachdb#80496

Release note (ui change): The job status page in the DB Console will now
show the status column for changefeed jobs and display the
`highwater_timestamp` value in a separate column. Thise more closely
matches the SQL output of `SHOW changefeed JOBS`. The highwater
timestamp now displays as the nanosecond system time value by default
with the human-readable value in the tooltip since the decimal value is
copy/pasted more often.
@dhartunian dhartunian force-pushed the improved-changefeed-status-db-console branch from f1a77a4 to a725eb2 Compare May 16, 2022 19:25
Copy link
Contributor

@abarganier abarganier left a comment

Choose a reason for hiding this comment

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

The code LGTM - my only suggestion is formatting the timestamp string to drop the trailing decimals. Or is there precedent elsewhere to display unix timestamps this way?

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @dhartunian and @HonoreDB)


pkg/ui/workspaces/db-console/src/views/jobs/highwaterTimestamp.tsx line 20 at r3 (raw file):

interface HighwaterProps {
  timestamp: ITimestamp;
  decimalString: string;

Can we format this as 1652819391140803000 instead of 1652819391140803000.0000000000?

Copy link
Collaborator Author

@dhartunian dhartunian left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @abarganier and @HonoreDB)


pkg/ui/workspaces/db-console/src/views/jobs/highwaterTimestamp.tsx line 20 at r3 (raw file):

Previously, abarganier (Alex Barganier) wrote…

Can we format this as 1652819391140803000 instead of 1652819391140803000.0000000000?

we could but it does show up with the dot in the SQL CLI output.

Copy link
Contributor

@abarganier abarganier left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @dhartunian and @HonoreDB)


pkg/ui/workspaces/db-console/src/views/jobs/highwaterTimestamp.tsx line 20 at r3 (raw file):

Previously, dhartunian (David Hartunian) wrote…

we could but it does show up with the dot in the SQL CLI output.

Gotcha - I guess best to keep things consistent then! Thanks for considering it.

@dhartunian
Copy link
Collaborator Author

@HonoreDB friendly ping, just want to confirm that the new screenshots reflect what you're expecting here.

Copy link
Contributor

@HonoreDB HonoreDB left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r3, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @dhartunian)

@dhartunian
Copy link
Collaborator Author

bors r=abarganier,HonoreDB

@craig
Copy link
Contributor

craig bot commented May 24, 2022

Build succeeded:

@craig craig bot merged commit 9a7dedb into cockroachdb:master May 24, 2022
@dhartunian dhartunian deleted the improved-changefeed-status-db-console branch November 14, 2022 16:37
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.

changefeeds, ui: changefeed status is not reported in db console when there's a checkpoint
5 participants