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

builtins: crdb_internal.request_statement_bundle should work with VIEWACTIVITY privilege #118759

Closed
xinhaoz opened this issue Feb 5, 2024 · 0 comments · Fixed by #118760
Closed

builtins: crdb_internal.request_statement_bundle should work with VIEWACTIVITY privilege #118759

xinhaoz opened this issue Feb 5, 2024 · 0 comments · Fixed by #118760
Assignees
@xinhaoz
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 5, 2024 •
edited by cockroach-jira-scripts
Loading

crdb_internal.request_statement_bundle currently requires the ADMIN or VIEWACTIVITY role option. In addition to the role option it should also allow th VIEWACTIVITY system priv.

Specifically, these calls should probably be replaced with HasGlobalPrivilegeOrRoleOption which checks for either the role or system privilege.

Jira issue: CRDB-35889
@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 5, 2024
@xinhaoz xinhaoz self-assigned this Feb 5, 2024
@xinhaoz xinhaoz mentioned this issue Feb 5, 2024
Merged
craig bot pushed a commit that referenced this issue Feb 6, 2024
@shralex @wenyihu6
@michae2 @xinhaoz @rail @rickystewart
Merge #117117 #117693 #118372 #118760 #118767 #118789 #118792 #118797
804d37e 
117117: kv: log slow requests on replica level in addition to range level r=shralex a=shralex

Previously, slow requests were only logged at the range level, but the logs did not indicate which replica is slow. Moreover, the SlowRPC metric attempted to represent the number of requests currently being retried, however it was done on the range level and therefore missed a second level of replica-level retries being done underneath.

This PR adds logging on the replica level, removes a confusing log line, and changes the metric to count the number of slow requests in a simpler manner.

Epic: https://cockroachlabs.atlassian.net/browse/CRDB-33510
Fixes: #114431

117693: changefeedccl: add observability metrics into sarama code r=rharding6373 a=wenyihu6

Now that this patch (#117544) has been merged, sarama now acknowledges and
reacts to kafka server's throttling messages by slowing down. To provide better
observability into sarama code, this patch adds a metrics registry interceptor
and a new metrics `changefeed.kafka_throttling_hist_nanos` which tracks time (in
nanos) spent in sarama's throttling when cockroachdb exceed the kafka quota.

Fixes: #117618

Release note: changefeed.kafka_throttling_hist_nanos has now been added to
metrics to monitor sarama throttling behavior resulting from exceeding kafka
quota.

118372: sql: fix flake in TestTxnContentionEventsTable r=yuzefovich a=michae2

In causeContention we deliberately hold a transaction open using pg_sleep to block an update statement. The timing we're trying to achieve is:

1. transaction insert
2. update starts and blocks
3. transaction held open using pg_sleep

We were using a WaitGroup to order (2) after (1), but there was no synchronization to ensure (3) came after (2).

This commit adds a retry loop that checks `crdb_internal.cluster_queries` to ensure (3) comes after (2).

Fixes: #118236

Release note: None

118760: builtins: allow VIEWACTIVITY priv to use crdb_internal.request_statem… r=xinhaoz a=xinhaoz

…ent_bundle

Previously only those with the VIEWACTIVITY role could use the crdb_internal.request_statement_bundle builtin. We should allow the VIEWACTIVITY privilege as well since role options are now deprecated. This allow also allow stmt bundle requests to be made from db-console for users with this granted privilege.

Epic: none
Fixes: #118759

Release note (bug fix): Those with VIEWACTIVITY privilege can now request statement bundles using crdb_internal.requets_statement_bundle or via db-console's sql activity page.

118767: release: confirm yum install r=celiala a=rail

This adds `-y` flag to install `yum` without user prompt.

Epic: none
Release note: None

118789: jobs,application_api: replace calls to `skip.Stress` with `skip.Duress` r=celiala a=rickystewart

`skip.Duress()` seems like it should have been used in this case as it gives more time under both `race` and `deadlock`. This will give these tests some extra time if they run in a heavyweight configuration but not "under stress".

Epic: CRDB-8308
Release note: None

118792: kvfollowerreadsccl: skip test under `race` not `stressrace` r=celiala a=rickystewart

Epic: CRDB-8308
Release note: None

118797: bincheck: do not run geos tests on Windows r=celiala a=rail

In #106642 we stopped shipping libgeos on Windows, but didn't update the bincheck test to reflect the change.

Epic: none
Release note: None

Co-authored-by: shralex <[email protected]>
Co-authored-by: Wenyi Hu <[email protected]>
Co-authored-by: Michael Erickson <[email protected]>
Co-authored-by: Xin Hao Zhang <[email protected]>
Co-authored-by: Rail Aliiev <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
@craig craig bot closed this as completed in ce81ca1 Feb 6, 2024
@blathers-crl blathers-crl bot mentioned this issue Feb 6, 2024
Merged
xinhaoz added a commit that referenced this issue Feb 6, 2024
@xinhaoz
builtins: allow VIEWACTIVITY priv to use crdb_internal.request_statem…
9e5c8e0 
…ent_bundle

Previously only those with the VIEWACTIVITY role could use the
crdb_internal.request_statement_bundle builtin. We should allow
the VIEWACTIVITY privilege as well since role options are now
deprecated. This allow also allow stmt bundle requests to be made
from db-console for users with this granted privilege.

Epic: none
Fixes: #118759

Release note (bug fix): Those with VIEWACTIVITY privilege can now
request statement bundles using crdb_internal.requets_statement_bundle
or via db-console's sql activity page.
wenyihu6 pushed a commit to wenyihu6/cockroach that referenced this issue Feb 21, 2024
@xinhaoz @wenyihu6
builtins: allow VIEWACTIVITY priv to use crdb_internal.request_statem…
062d268 
…ent_bundle

Previously only those with the VIEWACTIVITY role could use the
crdb_internal.request_statement_bundle builtin. We should allow
the VIEWACTIVITY privilege as well since role options are now
deprecated. This allow also allow stmt bundle requests to be made
from db-console for users with this granted privilege.

Epic: none
Fixes: cockroachdb#118759

Release note (bug fix): Those with VIEWACTIVITY privilege can now
request statement bundles using crdb_internal.requets_statement_bundle
or via db-console's sql activity page.
xinhaoz added a commit that referenced this issue Feb 23, 2024
@xinhaoz
builtins: allow VIEWACTIVITY priv to use crdb_internal.request_statem…
dd33011 
…ent_bundle

Previously only those with the VIEWACTIVITY role could use the
crdb_internal.request_statement_bundle builtin. We should allow
the VIEWACTIVITY privilege as well since role options are now
deprecated. This allow also allow stmt bundle requests to be made
from db-console for users with this granted privilege.

Epic: none
Fixes: #118759

Release note (bug fix): Those with VIEWACTIVITY privilege can now
request statement bundles using crdb_internal.requets_statement_bundle
or via db-console's sql activity page.
xinhaoz added a commit that referenced this issue Feb 23, 2024
@xinhaoz
builtins: allow VIEWACTIVITY priv to use crdb_internal.request_statem…
b268b65 
…ent_bundle

Previously only those with the VIEWACTIVITY role could use the
crdb_internal.request_statement_bundle builtin. We should allow
the VIEWACTIVITY privilege as well since role options are now
deprecated. This allow also allow stmt bundle requests to be made
from db-console for users with this granted privilege.

Epic: none
Fixes: #118759

Release note (bug fix): Those with VIEWACTIVITY privilege can now
request statement bundles using crdb_internal.requets_statement_bundle
or via db-console's sql activity page.
@xinhaoz xinhaoz mentioned this issue Feb 26, 2024
Merged
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Feb 26, 2024
@xinhaoz
builtins: allow VIEWACTIVITY priv to use crdb_internal.request_statem…
20b8ae6 
…ent_bundle

Previously only those with the VIEWACTIVITY role could use the
crdb_internal.request_statement_bundle builtin. We should allow
the VIEWACTIVITY privilege as well since role options are now
deprecated. This allow also allow stmt bundle requests to be made
from db-console for users with this granted privilege.

Epic: none
Fixes: cockroachdb#118759

Release note (bug fix): Those with VIEWACTIVITY privilege can now
request statement bundles using crdb_internal.requets_statement_bundle
or via db-console's sql activity page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@xinhaoz xinhaoz

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
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

builtins: allow VIEWACTIVITY priv to use crdb_internal.request_statem… xinhaoz/cockroach
1 participant
@xinhaoz