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

Regions/Nodes columns not showing up on Dedicated #85249

Open
maryliag opened this issue Jul 28, 2022 · 9 comments · Fixed by #88461
Open

Regions/Nodes columns not showing up on Dedicated #85249

maryliag opened this issue Jul 28, 2022 · 9 comments · Fixed by #88461
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-observability

Comments

@maryliag
Copy link
Contributor

maryliag commented Jul 28, 2022 •
edited by exalate-issue-sync bot
Loading

The column for Node/Regions are not being displayed on Statement Page. The filter of nodes is being displayed so the value must be correct, but some other check must be wrong, since it's not showing the column itself or on the column selector list.

To be clear, this column should not be displayed on Serverless

Jira issue: CRDB-18136

Epic CRDB-32130
@maryliag maryliag added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-observability labels Jul 28, 2022
@matthewtodd matthewtodd self-assigned this Sep 7, 2022
@matthewtodd
Copy link
Contributor

matthewtodd commented Sep 7, 2022 •
edited
Loading

@maryliag it looks like it's the logic to hide the Regions/Nodes column when the cluster's only running in one region that's causing this behavior. When I run the following command, I see the column as expected:

./cockroach demo \
  --insecure \
  --multitenant=false \
  --nodes=3 \
  --demo-locality=region=us-east1,az=1:region=us-central1,az=2:region=us-west1,az=3

So, this feature appears to be behaving as designed, unless I'm missing something?

@maryliag
Copy link
Contributor Author

maryliag commented Sep 7, 2022

on local it was working, but on cloud on our staging cluster it wasn't. Maybe it's related to it being busted, if you can confirm with another cluster on cloud that has multiple regions that this is working, you can go ahead and close this.

@matthewtodd
Copy link
Contributor

I'm looking around for a multi-region dedicated cluster in staging, but I haven't found one yet...

@matthewtodd
Copy link
Contributor

Found one, adwit-wi-aws1, with 3 regions / 3 machines.

The feature seems to be working as intended!

Screen Shot 2022-09-07 at 4 04 42 PM

@matthewtodd matthewtodd closed this as not planned Won't fix, can't repro, duplicate, stale Sep 7, 2022
@maryliag
Copy link
Contributor Author

maryliag commented Sep 7, 2022

@matthewtodd I see you're pointing to localhost, so maybe the master has the fix for it? If I go on the staging cluster that you pointed out, I don't see the column or the filter option

Screen Shot 2022-09-07 at 5 15 31 PM

@matthewtodd
Copy link
Contributor

Oof, thanks for the catch, @maryliag. Must be a difference with master, then? I'll see if there's something we need to backport to 22.1.

@matthewtodd matthewtodd reopened this Sep 8, 2022
@matthewtodd
Copy link
Contributor

Back on this today, and strangely I'm no longer seeing the column on master either. Digging back in now.

@maryliag
Copy link
Contributor Author

Back on this today, and strangely I'm no longer seeing the column on master either. Digging back in now.

the column won't show if you have only one node/region, so make sure you're testing with multiple node/regions

@matthewtodd
Copy link
Contributor

I think I found it, it looks like the page is assuming state.adminUI.nodes has been populated, but it hasn't. So we just need to dispatch the right action.

@matthewtodd matthewtodd mentioned this issue Sep 22, 2022
Merged
craig bot pushed a commit that referenced this issue Sep 23, 2022
@matthewtodd
Merge #88461
975701e 
88461: ui: sql activity pages depend on node info r=matthewtodd a=matthewtodd

Fixes #85249.

Previously, the statements, transactions, and transaction details pages had an implicit dependency on the `state.adminUI.nodes` list in the redux store, and without that list, the Regions/Nodes columns and filters would be incorrectly hidden for multiregion clusters, unless the user happened to have already browsed through a page (such as statement details) that loaded the list.

(This bug was affecting dedicated cloud clusters only. Self-hosted clusters were working properly because their global mechanism to sync alert data happened to [refresh the node list][1].)

This change makes the dependency explicit and refreshes the list of nodes on entering those three pages.

[1]: https://github.com/cockroachdb/cockroach/blob/6a25c6f4b985ddcd08ed51772f9e8a2645aa4582/pkg/ui/workspaces/db-console/src/redux/alerts.ts#L678-L682

| Before | After |
|--|--|
|<img width="1536" alt="Screen Shot 2022-09-22 at 10 42 33 AM" src="https://user-images.githubusercontent.com/5261/191789436-e2276010-5eeb-4fc6-8723-0e0581b7ac44.png">|<img width="1536" alt="Screen Shot 2022-09-22 at 10 41 12 AM" src="https://user-images.githubusercontent.com/5261/191789502-07461bea-ed09-4c85-a878-68edb0ffe7e7.png">|
|<img width="1536" alt="Screen Shot 2022-09-22 at 10 42 42 AM" src="https://user-images.githubusercontent.com/5261/191789576-ed08278c-3cb7-45ff-a719-dd5385f58e66.png">|<img width="1536" alt="Screen Shot 2022-09-22 at 10 41 22 AM" src="https://user-images.githubusercontent.com/5261/191789618-71433f0b-fbd1-4b48-bb86-41e12c869745.png">|
|<img width="1536" alt="Screen Shot 2022-09-22 at 10 43 00 AM" src="https://user-images.githubusercontent.com/5261/191789657-dd3cadc7-2757-417f-94b8-5a31ce760bde.png">|<img width="1536" alt="Screen Shot 2022-09-22 at 10 41 38 AM" src="https://user-images.githubusercontent.com/5261/191789688-0c4da2b1-0018-450c-931d-c24b70290425.png">|

Release note (bug fix): The statements, transactions, and transaction details pages in the admin console now properly show the Regions/Nodes columns and filters for multiregion clusters.

Co-authored-by: Matthew Todd <[email protected]>
@craig craig bot closed this as completed in 4fe7d93 Sep 23, 2022
blathers-crl bot pushed a commit that referenced this issue Sep 23, 2022
@matthewtodd
ui: sql activity pages depend on node info
a814372 
Fixes #85249.

Previously, the statements, transactions, and transaction details pages
had an implicit dependency on the `state.adminUI.nodes` list in the
redux store, and without that list, the Regions/Nodes columns and
filters would be incorrectly hidden for multiregion clusters, unless the
user happened to have already browsed through a page (such as statement
details) that loaded the list.

(This bug was affecting dedicated cloud clusters only. Self-hosted
clusters were working properly because their global mechanism to sync
alert data happened to [refresh the node list][1].)

This change makes the dependency explicit and refreshes the list of
nodes on entering those three pages.

[1]: https://github.com/cockroachdb/cockroach/blob/6a25c6f4b985ddcd08ed51772f9e8a2645aa4582/pkg/ui/workspaces/db-console/src/redux/alerts.ts#L678-L682

Release note (bug fix): The statements, transactions, and transaction
details pages in the admin console now properly show the Regions/Nodes
columns and filters for multiregion clusters.
@blathers-crl blathers-crl bot mentioned this issue Sep 23, 2022
Merged
@matthewtodd matthewtodd mentioned this issue Oct 12, 2022
Merged
matthewtodd added a commit that referenced this issue Oct 12, 2022
@matthewtodd
ui: sql activity pages depend on node info
481e1b9 
Fixes #85249.

Previously, the statements, transactions, and transaction details pages
had an implicit dependency on the `state.adminUI.nodes` list in the
redux store, and without that list, the Regions/Nodes columns and
filters would be incorrectly hidden for multiregion clusters, unless the
user happened to have already browsed through a page (such as statement
details) that loaded the list.

(This bug was affecting dedicated cloud clusters only. Self-hosted
clusters were working properly because their global mechanism to sync
alert data happened to [refresh the node list][1].)

This change makes the dependency explicit and refreshes the list of
nodes on entering those three pages.

[1]: https://github.com/cockroachdb/cockroach/blob/6a25c6f4b985ddcd08ed51772f9e8a2645aa4582/pkg/ui/workspaces/db-console/src/redux/alerts.ts#L678-L682

Release note (bug fix): The statements, transactions, and transaction
details pages in the admin console now properly show the Regions/Nodes
columns and filters for multiregion clusters.
@exalate-issue-sync exalate-issue-sync bot reopened this Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-observability
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ui: sql activity pages depend on node info
2 participants
@matthewtodd @maryliag