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

ui: db pages are using store ids as node ids for regions mapping #118957

Closed
xinhaoz opened this issue Feb 8, 2024 · 0 comments · Fixed by #119260
Closed

ui: db pages are using store ids as node ids for regions mapping #118957

xinhaoz opened this issue Feb 8, 2024 · 0 comments · Fixed by #119260
Assignees
Labels
A-observability-inf C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@xinhaoz
Copy link
Member

xinhaoz commented Feb 8, 2024

The databases pages display region information for databases and tables using a nodeId -> region map. The value currently being used to lookup region information is not the node id, but the store id. This is because the db details api returns the store ids replicas are located on, not the node ids. This bug likely remained undetected for a while since lots of clusters have 1-1 mapping from store id to node id but store ids and node ids cannot be used interchangeably. Clusters with multiple stores per node will see bugs where missing/incorrect region information is being reported.

See this comment for the table used to return db details on stores being used by the db -

// NB 2: All the values in the `*replicas` columns correspond to store IDs.

And here in the UI, where we use node id -> region mappings with the store id -

const nodes = dbStats?.replicaData.replicas || [];
const nodesByRegionString = getNodesByRegionString(
nodes,
nodeRegionsByID,
isTenant,
);

We can either create a store ids -> region map or map each store id to their node and use the existing map. Each node status response should contain the node's stores.

Jira issue: CRDB-35974

@xinhaoz xinhaoz added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-observability-inf A-observability-inf labels Feb 8, 2024
@xinhaoz xinhaoz self-assigned this Feb 8, 2024
@laurenbarker laurenbarker assigned laurenbarker and unassigned xinhaoz Feb 12, 2024
laurenbarker added a commit to laurenbarker/cockroach that referenced this issue Feb 15, 2024
Previously, store ids were being treated as node ids on the databases
view. This resulted in "Regions/Nodes" column showing inaccurate
information in some cases. Now, store ids are mapped to node ids on the
databases view which resolves the issue.

Fixes: cockroachdb#118957

Release note (ui change): Resolves an issue where clusters with multiple
stores per node may list inaccurate region/node information in the
databases table.
laurenbarker added a commit to laurenbarker/cockroach that referenced this issue Feb 21, 2024
Previously, store ids were being treated as node ids on the databases
view. This resulted in "Regions/Nodes" column showing inaccurate
information in some cases. Now, store ids are mapped to node ids on the
databases view which resolves the issue.

Fixes: cockroachdb#118957

Release note (ui change): Resolves an issue where clusters with multiple
stores per node may list inaccurate region/node information in the
databases table.
blathers-crl bot pushed a commit that referenced this issue Mar 11, 2024
Previously, store ids were being treated as node ids on the databases
view. This resulted in "Regions/Nodes" column showing inaccurate
information in some cases. Now, store ids are mapped to node ids on the
databases view which resolves the issue.

Also, a log message is skipped during tests so that the output isn't
polluted with expected warnings.

Fixes: #118957

Release note (ui change): Resolves an issue where clusters with multiple
stores per node may list inaccurate region/node information in the
databases table.
craig bot pushed a commit that referenced this issue Mar 11, 2024
119260: cluster-ui, db-console: fix bug where store ids are treated as node ids r=laurenbarker a=laurenbarker

Previously, store ids were being treated as node ids on the databases view. This resulted in "Regions/Nodes" column showing inaccurate information in some cases. Now, store ids are mapped to node ids on the databases view which resolves the issue.

Fixes: #118957

Release note (ui change): Resolves an issue where clusters with multiple stores per node may list inaccurate region/node information in the databases table.

120137: cluster-ui: fix insights page crash r=laurenbarker a=laurenbarker

When the Schema Insights Type is unitialized, the Schema Insights tab would crash because a string method was being called on `undefined`. The types are all correct, but because strict mode isn't enabled, it's not considered a type error.

Now the string method is only called if the Schema Insights Type is defined.

Fixes: https://cockroachlabs.atlassian.net/browse/CC-27391

Release note (bug fix): Fixes intermittent page crash on the Schema Insights view.

120157: *: various storage engine open refactors r=sumeerbhola a=jbowens

These commits are in preparation for decoupling engine and filesystem initialization.

**cli: adjust OpenEngine to take a RWMode enum**

This commit adjusts OpenEngine to take a new fs.RWMode enum indicating whether
the engine should be opened in read-only mode or read-write mode. Previously,
callers could pass an optional storage.ReadOnly option to opt into read-only
mode. Future work will require the filesystem itself be opened in either
read-write or read-only mode, requiring the 'ReadOnly' flag not be a
configuration option for the Engine but for the filesystem environment.

**batcheval,rangefeed: use storage.Open**

Use storage.Open rather than NewPebble when constructing engines for
benchmarks. In future work, NewPebble will be unexported with Open remaining
the one means of opening new engines.

Co-authored-by: Lauren Barker <[email protected]>
Co-authored-by: Jackson Owens <[email protected]>
@craig craig bot closed this as completed in e3c09d0 Mar 11, 2024
cockroach-dev-inf pushed a commit that referenced this issue Mar 11, 2024
Previously, store ids were being treated as node ids on the databases
view. This resulted in "Regions/Nodes" column showing inaccurate
information in some cases. Now, store ids are mapped to node ids on the
databases view which resolves the issue.

Also, a log message is skipped during tests so that the output isn't
polluted with expected warnings.

Fixes: #118957

Release note (ui change): Resolves an issue where clusters with multiple
stores per node may list inaccurate region/node information in the
databases table.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-observability-inf 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.

2 participants