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

dbconsole: console doesn't load for new clusters - interference with demo cookies? #95366

Closed
andreimatei opened this issue Jan 17, 2023 · 1 comment · Fixed by #95373
Closed

dbconsole: console doesn't load for new clusters - interference with demo cookies? #95366

andreimatei opened this issue Jan 17, 2023 · 1 comment · Fixed by #95373
Assignees
@knz
Labels
A-multitenancy Related to multi-tenancy C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-multitenant Issues owned by the multi-tenant virtual team

Comments

@andreimatei
Copy link
Contributor

andreimatei commented Jan 17, 2023 •
edited by cockroach-jira-scripts

I sometimes get in a state where the console doesn't load at all for a new cluster

rm -rf cockroach-data
./cockroach start-single-node --insecure --logtostderr

Go to http://127.0.0.1:8080/ - nothing loads. The Chrome Console shows:

Failed to load resource: the server responded with a status of 500 ()              chrome-error://chromewebdata/:1

Using an incognito window works; I believe that the breakage has something to do with having previously run a secure demo cluster (or a secure cluster in general perhaps) and having some cookies around from logging into that. Adding an --http-port=8081 and accessing on that port also works (without incognito), which tracks with this having something to do with cookies.
Interestingly, after accessing the page in incognito, accessing it from the non-incognito window seems to work again, until starting another demo cluster and logging into that.

Jira issue: CRDB-23485
@andreimatei andreimatei added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-multitenancy Related to multi-tenancy T-multitenant Issues owned by the multi-tenant virtual team labels Jan 17, 2023
@andreimatei
Copy link
Contributor Author

cc @dhartunian

@andreimatei andreimatei mentioned this issue Jan 17, 2023
Closed
@dhartunian dhartunian mentioned this issue Jan 17, 2023
Merged
craig bot pushed a commit that referenced this issue Jan 18, 2023
@xinhaoz @rafiss
@dhartunian @rail @yuzefovich
Merge #94720 #95258 #95373 #95397 #95444 #95447
1d2ba22 
94720: sql: add crdb_internal.{node,cluster}_txn_execution_insights r=xinhaoz a=xinhaoz

Closes #93075

This commit adds 2 new virtual tables displaying execution
insights for transaction.
- crdb_internal.cluster_txn_execution_insights
- crdb_internal.node_txn_execution_insights

Release note (sql change):
2 new virtual tables displaying execution insights for transaction:
- crdb_internal.cluster_txn_execution_insights
- crdb_internal.node_txn_execution_insights

----------------

Only the 2nd commit is relevant here.

95258: server: fix UserSQLRoles to account for global privileges r=maryliag a=rafiss

Epic: None
Release note (bug fix): DB Console features that check for the
VIEWACTIVITYREDACTED privilege now also account for global privileges.

95373: server: clear tenant cookies when tenant is missing r=knz a=dhartunian

Previously, when we had cookies in an HTTP Request referencing a tenant, and that tenant didn't exist, we'd be stuck in an error loop constantly attempting to start a tenant that didn't exist.

This commit updates the error log in `server_controller.go` to clear cookies for multi-tenant sessions when we return an error in order to break the browser out of the bad update cycle and have it either continue happily with an insecure cluster that doesn't need a session anyway, or request a new one by routing the user to the login page.

Resolves: #95365
Resolves: #95366
Epic: CRDB-14545

Release note: None

95397: ui: fix sql stats page data refreshing r=xinhaoz a=xinhaoz

This commit fixes a couple of bugs with sql stats page data fetching.

First - we missed clearing the initial timeout to fetch data that was created on mount - it should be cleared when a new time range is selected on the page or when the page is unmounted. Currently, this bug can lead to unexpeted data refetches of the incorrect (previously selected) time range.

Second - with the introduction of new pages that use the time picker, such as insights, sql stats pages are no longer the only page that can change the time range. Previously, data was only fetched on mount for sql stats pages if it was not previously fetched, or a data refresh was scheduled based on last update time if the current time range was not a custom (fixed) range. We should now also refetch when the data is outdated due to a new time range being selected on a different page. This commit introduces the `isDataValid` prop to sql stats pages to determine this and refetch data accordingly.

Epic: none


CC - just showing all the fetching is still working:
https://www.loom.com/share/1b24ec65dbe64dcd9c23cd2be980b877

DB Console -
https://www.loom.com/share/0afe1722d7ab418bb7f41c2462f4d84a

95444: ci: publish `latest-master-build` docker image r=jlinder a=rail

Previously, our automation published latest docker images using `latest-v2x.x-build` tag. External automation that tracked the master branch would need to update the tag, when we change the version.

This PR adds an extra step to tag the master branch builds with `latest-master-build` docker tag.

Epic: none
Release note: None

95447: sem/builtins: fix population of generator builtins r=yuzefovich a=yuzefovich

Generator builtins are such that they should not evaluated as scalars (i.e. `Fn` and `FnWithExprs` evaluation functions should not be called on them). In order to highlight this as an assertion failure we initialize generator builtins with special functions in those two fields. However, previously the initialization was broken since we were modifying a copy of the builtin struct, and this is now fixed. There isn't much of a production impact though (if we ever tried to evaluate the generator as a scalar, it'd result in a nil pointer which would be caught by the vectorized engine panic-catcher; also, it seems very hard if possible to trigger this without the testing randomizations), so there is no release note.

Informs: #94890.
Epic: None

Release note: None

Co-authored-by: Xin Hao Zhang <xzhang@cockroachlabs.com>
Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
Co-authored-by: David Hartunian <davidh@cockroachlabs.com>
Co-authored-by: Rail Aliiev <rail@iqchoice.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
@craig craig bot closed this as completed in 9fbd5da Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@knz knz

Labels
A-multitenancy Related to multi-tenancy C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-multitenant Issues owned by the multi-tenant virtual team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

server: clear tenant cookies when tenant is missing dhartunian/cockroach
2 participants
@andreimatei @knz