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

ui: back off making new requests if receiving a 504 #90596

Closed
ajwerner opened this issue Oct 25, 2022 · 0 comments · Fixed by #90862
Closed

ui: back off making new requests if receiving a 504 #90596

ajwerner opened this issue Oct 25, 2022 · 0 comments · Fixed by #90862
Assignees
@maryliag
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@ajwerner
Copy link
Contributor

ajwerner commented Oct 25, 2022 •
edited by cockroach-jira-scripts
Loading

Is your feature request related to a problem? Please describe.
I observe that some endpoints time out at 30s and get a 504. I then observe the front-end immediately re-issuing those requests. A concrete example is trying to fetch privileges or maybe just table details on the database details page.

Describe the solution you'd like
The page out to back off up to some number of minutes between making the requests if it is going to re-issue them at all.

Describe alternatives you've considered
Don't retry and move into an error state.

Jira issue: CRDB-20853
@ajwerner ajwerner added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Oct 25, 2022
@ajwerner ajwerner mentioned this issue Oct 26, 2022
Merged
@maryliag maryliag self-assigned this Oct 28, 2022
@maryliag maryliag mentioned this issue Oct 28, 2022
Merged
craig bot pushed a commit that referenced this issue Oct 31, 2022
@maryliag @ajwerner
Merge #90488 #90862 #90962
c476f0d 
90488: sql,descs: add & adopt descriptor_validation session var r=postamar a=postamar

This commit removes the `sql.catalog.descs.validate_on_write.enabled` cluster setting and replaces it with the `descriptor_validation` session variable. The default value is 'on' which indicates that catalog descriptors are to be validated when both read from- and written to the system.descriptor table. Other possible values are 'off' which disables validation entirely and 'read_only' which disables it for writes.

Informs #50651.

Release note (sql change): added a new 'descriptor_validation' session variable which can be set to 'read_only' or 'off' to disable descriptor validation, which may be useful when mitigating or recovering from catalog corruption.

90862: ui: handle errors on db endpoints r=maryliag a=maryliag

Previously, when hitting an error on endpoints
used on the database page, we would just keep retrying constantly, without showing a proper error state.
On SQL Activity page, for example, we show the error message and let the user retry if they want.
This commit uses the same logic on the Database page. Since the pages make several requests and just part of them can fail, some of the pages we will still load, but give a warning about unavailable data and show the error message about reload option.

This commit also increases timeout of database endpoints.

Fixes #90596

<img width="636" alt="Screen Shot 2022-10-28 at 6 28 55 PM" src="https://user-images.githubusercontent.com/1017486/198745467-7833de82-4eac-41fe-85ef-5035f99b0f35.png">


<img width="866" alt="Screen Shot 2022-10-28 at 6 29 30 PM" src="https://user-images.githubusercontent.com/1017486/198745476-306ce1af-9476-4d47-9f9d-46c954e69ab8.png">


https://www.loom.com/share/edc46386a6fe408b90fa5b6330870819

Release note: None

90962: sql: fix newly introduced race r=ajwerner a=ajwerner

All of the goroutines were writing to the same `err` variable. This race was introduced in #89540.

Epic: None

Release note: None

Co-authored-by: Marius Posta <[email protected]>
Co-authored-by: maryliag <[email protected]>
Co-authored-by: Andrew Werner <[email protected]>
@craig craig bot closed this as completed in 35caa9e Oct 31, 2022
@maryliag maryliag mentioned this issue Oct 31, 2022
Merged
maryliag added a commit to maryliag/cockroach that referenced this issue Oct 31, 2022
@maryliag
ui: handle errors on db endpoints
86dcb5c 
Previously, when hitting an error on endpoints
used on the database page, we would just keep retrying
constantly, without showing a proper error state.
On SQL Activity page, for example, we show the error
message and let the user retry if they want.
This commit uses the same logic on the Database page.
Since the pages make several requests and just part of
them can fail, some of the pages we will still load, but
give a warning about unavailable data and show the error
message about reload option.

This commit also increases timeout of database endpoints.

Fixes cockroachdb#90596

Release note: None
@maryliag maryliag mentioned this issue Oct 31, 2022
Merged
maryliag added a commit to maryliag/cockroach that referenced this issue Oct 31, 2022
@maryliag
ui: handle errors on db endpoints
d29f8e0 
Previously, when hitting an error on endpoints
used on the database page, we would just keep retrying
constantly, without showing a proper error state.
On SQL Activity page, for example, we show the error
message and let the user retry if they want.
This commit uses the same logic on the Database page.
Since the pages make several requests and just part of
them can fail, some of the pages we will still load, but
give a warning about unavailable data and show the error
message about reload option.

This commit also increases timeout of database endpoints.

Fixes cockroachdb#90596

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@maryliag maryliag

Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ui: handle errors on db endpoints maryliag/cockroach
2 participants
@maryliag @ajwerner