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

remove error once page is changed #97533

Closed
maryliag opened this issue Feb 22, 2023 · 0 comments · Fixed by #103240
Closed

remove error once page is changed #97533

maryliag opened this issue Feb 22, 2023 · 0 comments · Fixed by #103240
Assignees
Labels
A-sql-observability Related to observability of the SQL layer C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@maryliag
Copy link
Contributor

maryliag commented Feb 22, 2023

currently if we hit a crash on a page, we show an error message, but even after clicking on other pages using the side nav, the error message is still displayed.
The message should no longer show when moving to another page, without the need to do a force refresh

Jira issue: CRDB-24742

@maryliag maryliag added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-sql-observability Related to observability of the SQL layer T-sql-observability labels Feb 22, 2023
@maryliag maryliag self-assigned this May 12, 2023
craig bot pushed a commit that referenced this issue May 15, 2023
102772: pkg/util/log: unify explicit flush of network and file sinks r=knz a=abarganier

Ref: #101562

both for files *and* network sinks, such as fluent-servers.

I received some feedback that we shouldn't divide these flush operations based on sink type, and instead we should unify the flush operation to flush both (as the crash reporter already does).

The existing function to flush just the file sinks is quite widely used. Introducing flushing of network sinks begs the question, "What if this adds to the runtime of code using this explicit flush mechanism?"

Well, the existing FlushFileSinks calls fsync() [1] with F_FULLFSYNC [2]. IIUC, this means that it already is a blocking operation as the fsync() call will wait for the buffered data to be written to permanent storage (not just the disk cache). Given that, I think any caller of this function already assumes it's a blocking operation and therefore should be tolerant of that.

[1]: https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fsync.2.html
[2]: https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fcntl.2.html#//apple_ref/doc/man/2/fcntl

Nonetheless, we implement a timeout mechanism for the flushing of the buffered network sinks as an added protection.

Fixes: #101369

102785: codeowners, roachtest: merge sql-schema, sql-sessions to sql-foundations r=celiala a=celiala

Reflecting merge sql-schema, sql-sessions to sql-foundations in:

- [x] github CODEOWNERS
- [x] pkg/cmd/roachtest owners
- [x] TEAMS.yaml

Tests should pass once new team is created via https://github.com/cockroachlabs/crl-infrastructure/pull/775

This is a task for Part 2, below.
____

### Tasks to update: GitHub Team name + GitHub Projects + Blathers

Part 1
- [x] Create GitHub T- label: https://github.com/cockroachdb/cockroach/labels/T-sql-foundations
- [ ] Create new sql-foundations GitHub team name: https://github.com/cockroachlabs/crl-infrastructure/pull/775

Part 2

- [ ] Update Blathers: https://github.com/cockroachlabs/blathers-bot/pull/123
- [ ] Update CODEOWNERS, roachtest, TEAMS.yaml:
  - [ ] master: #102785
  - [ ] release-23.1: #102924 
  - [ ] release-22.2: #102925
- [ ] [Manual Step] To be done at same time as merging Part 2 PRs:
  - [ ] Manually rename "SQL Schema" Project to "SQL Foundations": https://github.com/cockroachdb/cockroach/projects/47

Part 3

- [ ] Remove old GitHub team names: https://github.com/cockroachlabs/crl-infrastructure/pull/776

____


Partial work for DEVINFHD-867
Release note: None
Epic: DEVINFHD-867


103118: jwtauthccl: allow cluster SSO to pick principals from custom claims r=kpatron-cockroachlabs a=kpatron-cockroachlabs

Previously, cluster SSO (JWT authentication) required user principals to be in the subject field of the JWT, which by the JWT specification required them to be a single string. A customer has a desire have which SQL users a human is allowed to login as be specified by their IDP via a "groups" field in the JWT.

To accomadate this, a new cluster setting has been introduced (server.jwt_authentication.claim), which when populated specifies which claim of the JWT to read as containing the principal. This value can be either a string or an array of strings. The resulting principal(s) are then fed through the identity map as before to produce the set of valid SQL users that this token can login as. As before, humans can specify which SQL user they wish to login with via the username field as long as it matches one of the values the token is a valid authentication method for.

Fixes: CC-24595

Release note (enterprise change): Cluster SSO (JWT authentication) can now read SQL usernames from any JWT claims instead of requiring the subject claim to be used.

103240: ui: no need to refresh page after error r=maryliag a=maryliag

Previously, if one page crashes on DB Console, all other pages would show the same error message and the user had to force a refresh on the browser to be able to see the other pages. Now only the broken page shows the error and all the other pages load as expected. The user still needs to force a refresh on the broken page if they want to retry.

Fixes #97533

https://www.loom.com/share/56a6d811d9604b7abe673c1430ee605e

Release note (ui change): If a page crashed, a force refresh is no longer required to be able to see the other pages on DB Console.

103301: backupccl: disable restore data processor memory monitoring by default r=rhu713 a=rhu713

Temporarily disable memory monitoring for the restore data processor due to current logic not handling deletes correctly.

Epic: none

Co-authored-by: Alex Barganier <[email protected]>
Co-authored-by: Celia La <[email protected]>
Co-authored-by: Kyle Patron <[email protected]>
Co-authored-by: maryliag <[email protected]>
Co-authored-by: Rui Hu <[email protected]>
@craig craig bot closed this as completed in d7d77e3 May 15, 2023
blathers-crl bot pushed a commit that referenced this issue May 15, 2023
Previously, if one page crashes on DB Console, all other
pages would show the same error message and the user had to
force a refresh on the browser to be able to see the other pages.
Now only the broken page shows the error and all the other pages
load as expected. The user still needs to force a refresh on
the broken page if they want to retry.

Fixes #97533

Release note (ui change): If a page crashed, a force refresh is
no longer required to be able to see the other pages on DB Console.
blathers-crl bot pushed a commit that referenced this issue May 15, 2023
Previously, if one page crashes on DB Console, all other
pages would show the same error message and the user had to
force a refresh on the browser to be able to see the other pages.
Now only the broken page shows the error and all the other pages
load as expected. The user still needs to force a refresh on
the broken page if they want to retry.

Fixes #97533

Release note (ui change): If a page crashed, a force refresh is
no longer required to be able to see the other pages on DB Console.
blathers-crl bot pushed a commit that referenced this issue May 15, 2023
Previously, if one page crashes on DB Console, all other
pages would show the same error message and the user had to
force a refresh on the browser to be able to see the other pages.
Now only the broken page shows the error and all the other pages
load as expected. The user still needs to force a refresh on
the broken page if they want to retry.

Fixes #97533

Release note (ui change): If a page crashed, a force refresh is
no longer required to be able to see the other pages on DB Console.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-observability Related to observability of the SQL layer C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant