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

tablemetadatacache,ui: change error messaging for failed tmj update #134902

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

kyle-a-wong
Copy link
Contributor

@kyle-a-wong kyle-a-wong commented Nov 11, 2024

Previously, error messages from the table metaddata update job were surfaced in the UI on the table overview page. These errors are usually overly verbose and don't provide any actionable info to the end user.

In addition to this, error messages can be very big if the error is a result of a span stats fanout failure, which can contain errors for each fanned out request.

Now, db-console will simply report that an error occured in the job and that the data is stale. The error returned from failed SpanStats requests will now have a generic "An error has occurred while fetching span stats."

Epic: None
Release note: None

Old error messaging:
image

New error messaging:
image

@kyle-a-wong kyle-a-wong requested a review from a team as a code owner November 11, 2024 22:39
@kyle-a-wong kyle-a-wong requested review from xinhaoz and removed request for a team November 11, 2024 22:39
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@angles-n-daemons angles-n-daemons left a comment

Choose a reason for hiding this comment

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

one small request, looks good otherwise

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @dhartunian, @kyle-a-wong, and @xinhaoz)


-- commits line 4 at r1:
spelling: metaddata


pkg/sql/tablemetadatacache/table_metadata_batch_iterator.go line 206 at r1 (raw file):

	if len(res.Errors) > 0 {
		// For now, we won't write partial results to the cache.

Can we log the errors, so if something is failing repeatedly we could see the actual error message?

eg:

for err := range res.Errors {
    log.Error(ctx, err)
}

Previously, error messages from the table metadata update job
were surfaced in the UI on the table overview page. These errors
are usually overly verbose and don't provide any actionable info
to the end user.

In addition to this, error messages can be very big if the error
is a result of a span stats fanout failure, which can contain
errors for each fanned out request.

Now, db-console will simply report that an error occured in the
job and that the data is stale. The error returned from failed
SpanStats requests will now have a generic "An error has occurred
while fetching span stats."

Epic: None
Release note: None
Copy link
Contributor Author

@kyle-a-wong kyle-a-wong 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 @angles-n-daemons, @dhartunian, and @xinhaoz)


pkg/sql/tablemetadatacache/table_metadata_batch_iterator.go line 206 at r1 (raw file):

Previously, angles-n-daemons (Brian Dillmann) wrote…

Can we log the errors, so if something is failing repeatedly we could see the actual error message?

eg:

for err := range res.Errors {
    log.Error(ctx, err)
}

Done

Copy link
Contributor Author

@kyle-a-wong kyle-a-wong 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 @angles-n-daemons, @dhartunian, and @xinhaoz)


-- commits line 4 at r1:

Previously, angles-n-daemons (Brian Dillmann) wrote…

spelling: metaddata

done

Copy link
Contributor

@angles-n-daemons angles-n-daemons 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 @xinhaoz)

@kyle-a-wong
Copy link
Contributor Author

tftr!

bors r+

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