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.

Sign up for GitHub

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

Jump to bottom

release-24.1: dbconsole,sql: remove internal cols from jobs table, SHOW #121286

Merged
merged 4 commits into from Mar 28, 2024
Merged

release-24.1: dbconsole,sql: remove internal cols from jobs table, SHOW #121286

merged 4 commits into from Mar 28, 2024

Conversation

blathers-crl[bot]
Copy link

@blathers-crl blathers-crl bot commented Mar 28, 2024 •
edited by dt

Backport 1/1 commits from #121269, 3/3 from #121272 on behalf of @dt.

/cc @cockroachdb/release

These are internal jobs system state that few jobs use. These values are visible on the per-job detail page if one is interested but are just noise on the table, and drawing attention to them can be misleading as most jobs do retries themselves causing these numbers to be incorrect/irrelevant at best.

Release note (ui change): the jobs table page no longer includes two columns related to a deprecated internal implementation detail (last execution time and execution count).

Epic: none.

Release justification: cleanup that was in the bors queue when branch was cut.

@dt
dbconsole: remove internal cols from jobs table
e3799a8 
These are internal jobs system state that few jobs use. These values are visible on the
per-job detail page if one is interested but are just noise on the table, and drawing
attention to them can be misleading as most jobs do retries themselves causing these
numbers to be incorrect/irrelevant at best.

Release note (ui change): the jobs table page no longer includes two columns related to a
deprecated internal implementation detail (last execution time and execution count).

Epic: none.
@blathers-crl blathers-crl bot requested a review from a team as a code owner March 28, 2024 16:46
@blathers-crl blathers-crl bot force-pushed the blathers/backport-release-24.1-121269 branch from c8fe91a to e3799a8 Compare March 28, 2024 16:46
@blathers-crl blathers-crl bot added blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot. labels Mar 28, 2024
@blathers-crl blathers[crl]
Copy link
Author

blathers-crl bot commented Mar 28, 2024

Thanks for opening a backport.

Please check the backport criteria before merging:

  • Backports should only be created for serious
    issues     or test-only changes.
  • Backports should not break backwards-compatibility.
  • Backports should change as little code as possible.
  • Backports should not change on-disk formats or node communication protocols.
  • Backports should not add new functionality (except as defined
    here).
  • Backports must not add, edit, or otherwise modify cluster versions; or add version gates.
  • All backports must be reviewed by the owning areas TL and one additional
    TL. For more information as to how that review should be conducted, please consult the backport
    policy    .
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
  • There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
  • The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
  • New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters. State changes must be further protected such that nodes running old binaries will not be negatively impacted by the new state (with a mixed version test added).
  • The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.
  • Your backport must be accompanied by a post to the appropriate Slack
    channel (#db-backports-point-releases or #db-backports-XX-X-release) for awareness and discussion.

Also, please add a brief release justification to the body of your PR to justify this
backport.

@blathers-crl blathers-crl bot added the backport Label PR's that are backports to older release branches label Mar 28, 2024
@cockroach-teamcity
Copy link
Member

This change is Reviewable

dt added 3 commits March 28, 2024 17:53
@dt
sql: remove internal retry state from SHOW JOBS
d0e06cf 
These are only used by a couple of job types and are actively misleading for
all the other job types that do retries differently. Presenting them in the
SHOW JOBS output is at best noise, and potentially actively harmful if it
causes one to dismiss a job as not retrying. These internal counters/times
are only meaningful to someone who knows the jobs system internals and we
know where to find them in the system table.

While I'm here: trace ID and exec errors aren't adding anything to the table
of all josb, so only show them when inspecting specific jobs.

Release note (sql change): SHOW JOBS no longer shows some internal retry
counter columns (`last_run`, `next_run`, `num_runs`) and now only shows the
`statement`, 'trace_id` and `execution_errors` columns when inspecting a
specific job ID or IDs.

Epic: none.
@dt
sql: use second granularity in SHOW JOBS
783311e 
Printing these with 6 decimal places was adding noise to a table meant for humans to
consume, with times on human scales, e.g. an hour ago vs a second ago. The raw times
are available in internal tables if/when we need to do involved debugging but that is
not the purpose of SHOW JOBS.

Release note: none.
Epic: none.
@dt
sql: shorted description in SHOW JOBS
51c53a6 
Release note (sql change): SHOW JOBS now shortens long job descriptions to 100 characters to make the table easier to read while the full description and statement can be inspected using SHOW JOB or SHOW JOBS on specific Job IDs.
Epic: none.
@blathers-crl blathers-crl bot requested review from a team as code owners March 28, 2024 17:54
@blathers-crl blathers-crl bot requested review from rharding6373 and michae2 and removed request for a team March 28, 2024 17:54
@dt dt changed the title release-24.1: dbconsole: remove internal cols from jobs table release-24.1: dbconsole,sql: remove internal cols from jobs table, SHOW Mar 28, 2024
stevendanna
stevendanna approved these changes Mar 28, 2024
View reviewed changes
Copy link
Collaborator

@stevendanna stevendanna left a comment

Choose a reason for hiding this comment

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

While this wouldn't be a backportable change for a released version, this change was approved and ready to merge before the branch cut and simply got caught up in a bors queue.

As such, I think it makes sense to backport it rather than delaying it for another release.

The change itself is fairly low risk to other parts of the system.

@dt dt merged commit af87939 into release-24.1 Mar 28, 2024
5 of 6 checks passed
@dt dt deleted the blathers/backport-release-24.1-121269 branch March 28, 2024 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stevendanna stevendanna stevendanna approved these changes

@rharding6373 rharding6373 rharding6373 approved these changes

@abarganier abarganier Awaiting requested review from abarganier

@dhartunian dhartunian Awaiting requested review from dhartunian

@dt dt Awaiting requested review from dt

@michae2 michae2 Awaiting requested review from michae2 michae2 is a code owner automatically assigned from cockroachdb/sql-queries-prs

Assignees

@dt dt

Labels
backport Label PR's that are backports to older release branches blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@cockroach-teamcity @stevendanna @rharding6373 @dt