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

insights: cannot read properties of undefined (reading 'split') #99539

Closed
gtr opened this issue Mar 24, 2023 · 0 comments · Fixed by #99962
Closed

insights: cannot read properties of undefined (reading 'split') #99539

gtr opened this issue Mar 24, 2023 · 0 comments · Fixed by #99962
Labels
A-cluster-observability Related to cluster observability C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@gtr
Copy link
Contributor

gtr commented Mar 24, 2023 •
edited
Loading

Describe the problem

A type error was encountered on the SQL Activity page on serverless.

To Reproduce

I'm unable to view the replay but it appears that soon after a route change to /insights, the user encountered the following error:

TypeError: Cannot read properties of undefined (reading 'split')
  at q.isOptionSelected @ https://cockroachlabs.cloud/assets/admin-ui-22-2.3f732429d80a4fa60862.js:1495:5832
  at <anonymous> @ https://cockroachlabs.cloud/assets/admin-ui-22-2.3f732429d80a4fa60862.js:1495:7241
  at Array.map @ <anonymous>
  at q.render @ https://cockroachlabs.cloud/assets/admin-ui-22-2.3f732429d80a4fa60862.js:1495:7197
  at Dl @ https://cockroachlabs.cloud/assets/common.29615ddb65bbd14ef358.js:2498:68424
  at Nl @ https://cockroachlabs.cloud/assets/common.29615ddb65bbd14ef358.js:2498:68203
  at Ji @ https://cockroachlabs.cloud/assets/common.29615ddb65bbd14ef358.js:2502:10265
  at fr @ https://cockroachlabs.cloud/assets/common.29615ddb65bbd14ef358.js:2502:996
  at He @ https://cockroachlabs.cloud/assets/common.29615ddb65bbd14ef358.js:2502:924
  at tn @ https://cockroachlabs.cloud/assets/common.29615ddb65bbd14ef358.js:2502:774

Based on the stack trace, it most likely has to do with either the txn or stmt insight selectors. isOptionSelected suggests it may be from a column selector.

Additional data / screenshots

Environment:

  • CockroachDB version: Cockroach Serverless
  • Server OS: Mac OS X 10.12.7

Jira issue: CRDB-25920
@gtr gtr added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-cluster-observability A-cluster-observability Related to cluster observability labels Mar 24, 2023
maryliag added a commit to maryliag/cockroach that referenced this issue Mar 29, 2023
@maryliag
ui: add checks for values
2b9fd25 
Fixes cockroachdb#99655
Fixes cockroachdb#99538
Fixes cockroachdb#99539

Add checks to usages that could cause
`Cannot read properties of undefined`.

Release note: None
@maryliag maryliag mentioned this issue Mar 29, 2023
Merged
craig bot pushed a commit that referenced this issue Mar 30, 2023
@btkostner @knz
@maryliag @yuzefovich @nicktrav
Merge #98899 #99607 #99690 #99750 #99962 #99963
331a672 
98899: feat: allow starting docker container via env variable r=rickystewart a=btkostner

Fixes #87043 by allowing you to specify args via the `COCKROACH_ARGS` env value instead of a command. This is required to be able to use the official cockroach image with GitHub actions via a service. More details in the issue.

Note, do to weirdness with merging commands and env values, I decided that setting the `COCKROACH_ARGS` would ignore any command given. This should reduce issues with people trying to use both ways of specifying args and instead force them to pick one.

Release note (general change): Allow setting docker command args via the `COCKROACH_ARGS` environment variable.

99607: sql: block DROP TENANT based on a session var r=stevendanna a=knz

Fixes #97972.
Epic: CRDB-23559

In clusters where we will promote tenant management operations, we would like to ensure there is one extra step needed for administrators to drop a tenant (and thus irremedially lose data). Given that `sql_safe_updates` is not set automatically when users open their SQL session using their own client, we need another mechanism.

This change introduces the new (hidden) session var, `disable_drop_tenant`. When set, tenant deletion fails with the following error message:

```
demo@127.0.0.1:26257/movr> drop tenant foo;
ERROR: rejected (via sql_safe_updates or disable_drop_tenant): DROP TENANT causes irreversible data loss
SQLSTATE: 01000
```

(The session var `sql_safe_updates` is _also_ included as a blocker in the mechanism so that folk using `cockroach sql` get double protection).

The default value of this session var is `false` in single-tenant clusters, for compatibility with CC Serverless. It will be set to `true` via a config profile (#98466) when suitable.

Release note: None

99690: ui: drop index with space r=maryliag a=maryliag

Previously, if the index had a space on its name,
it would fail to drop.
This commit adds quotes so it can be executed.

Fixes #97988

Schema Insights
https://www.loom.com/share/04363b7f83484b5da19c760eb8d0de21

Table Details page
https://www.loom.com/share/1519b897a14440ddb066fb2ab03feb2d

Release note (bug fix): Index recommendation to DROP an index that have a space on its name can now be properly executed.

99750: sql: remove no longer used channel in createStatsNode r=yuzefovich a=yuzefovich

This hasn't been used as of fe6377c. Also mark `create_stats.go` as owned by SQL Queries.

Epic: None

Release note: None

99962: ui: add checks for values r=maryliag a=maryliag

Fixes #99655
Fixes #99538
Fixes #99539

Add checks to usages that could cause
`Cannot read properties of undefined`.

Release note: None

99963: roachtest: use local SSDs for disk-stall failover tests r=andrewbaptist a=nicktrav

The disk-stalled roachtests were updated in #99747 to use local SSDs. This change broke the `failover/*/disk-stall` tests, which look for `/dev/sdb` on GCE (the used for GCE Persistent Disks), but the tests still create clusters with local SSDs (the roachtest default).

Fix #99902.
Fix #99926.
Fix #99930.

Touches #97968.

Release note: None.

Co-authored-by: Blake Kostner <git@btkostner.io>
Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
Co-authored-by: maryliag <marylia@cockroachlabs.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Nick Travers <travers@cockroachlabs.com>
@craig craig bot closed this as completed in 119c54d Mar 30, 2023
@blathers-crl blathers-crl bot mentioned this issue Mar 30, 2023
Merged
blathers-crl bot pushed a commit that referenced this issue Mar 30, 2023
@maryliag
ui: add checks for values
2534915 
Fixes #99655
Fixes #99538
Fixes #99539

Add checks to usages that could cause
`Cannot read properties of undefined`.

Release note: None
maryliag added a commit to maryliag/cockroach that referenced this issue Mar 30, 2023
@maryliag
ui: add checks for values
312ddcc 
Fixes cockroachdb#99655
Fixes cockroachdb#99538
Fixes cockroachdb#99539

Add checks to usages that could cause
`Cannot read properties of undefined`.

Release note: None
@maryliag maryliag mentioned this issue Mar 30, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-cluster-observability Related to cluster observability C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ui: add checks for values maryliag/cockroach
1 participant
@gtr