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

server: allow configuring vmodule cluster-wide #89298

Closed
dt opened this issue Oct 4, 2022 · 0 comments · Fixed by #89648
Closed

server: allow configuring vmodule cluster-wide #89298

dt opened this issue Oct 4, 2022 · 0 comments · Fixed by #89648
Assignees
Labels
A-cli-server CLI commands that pertain to CockroachDB server processes A-observability-inf C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-server-and-security DB Server & Security

Comments

@dt
Copy link
Member

dt commented Oct 4, 2022

setting the vmodule for some component is often very useful when debugging, but is sometimes difficult in large clusters, particularly when the interaction with them is through a load-balancer, as the builtin that can be used to set it at runtime only manipulates the in-memory state on the node that executes it.

We could augment the current setup by adding a cluster setting string such as cluster.debug.vmodule that can be set to the same string one would provide to set_vmodule. This cluster setting would have an on-change hook such that when it is updated, the value is applied, meaning all nodes update their vmodule when it is set or changed.

Direct mutation of a single node's vmodule would still be possible via set_vmodule or the cli flag, overriding anything applied via the setting until the setting is next changed.

Jira issue: CRDB-20195

@dt dt added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-cli-server CLI commands that pertain to CockroachDB server processes T-observability-inf labels Oct 4, 2022
@dt dt self-assigned this Oct 4, 2022
blathers-crl bot pushed a commit that referenced this issue Oct 18, 2022
Closes #89298.

Release note: none.

Epic: none.
blathers-crl bot pushed a commit that referenced this issue Oct 18, 2022
Closes #89298.

Release note: none.

Epic: none.
craig bot pushed a commit that referenced this issue Oct 26, 2022
84888: distsql: remove queueing in the flow scheduler r=yuzefovich a=yuzefovich

**flowinfra: de-duplicate cluster flow tests**

Previously, there was a lot of duplication for two cluster flow tests
(one for the single tenant setup and another for the multi tenant
setup), and this commit cleans it up.

Release note: None

**distsql: remove queueing in the flow scheduler**

This commit removes the queueing behavior in the flow scheduler (which
is now renamed to "remote flow runner" to better reflect its purpose).
This behavior was already disabled on 22.2 (with a cluster setting that
could enable it back as an escape hatch) since we wanted to be
conservative when removing it, but I don't foresee any problems with
this, so it should be safe to remove it.

We don't remove the "cancel dead flow coordinator" since it might still
be useful in a mixed-version cluster, but more importantly the
coordinator will be refactored to also cancel the running flows (not
just the queued flows as it it used to). (This change will be in
a separate commit.)

This required removal some of the tests that relied on the queueing
behavior, but I don't think we're losing much test coverage.

Fixes: #34229.

Release note (sql change): `sql.distsql.max_running_flows` cluster
setting has been removed. This setting previously controlled the number
of remote DistSQL flows that a single node would run at any time. Once
that number was exceeded, the incoming flows would get queued until the
number was reduced. This was used as a poor man's version of the
admission control, but now that we have an actual admission control in
place, we don't need that queueing behavior.

89324: roachtest: add admissioncontrol/index-overload r=andrewbaptist a=andrewbaptist

This test sets up a 3 node cluster and measures the impact of creating
an index while a controlled KV workload is running. The test measures
two things
* The baseline KV workload P99 latency
* The impact on running index creation on the workload.

The KV workload is designed to use about 20% of the CPU and IO
resources of the system. Index creation is impactful by both reading
a lot of data and writing a large index, however the primary impact
is that it causes enough L0 inversion to make user traffic pause.

Release note: None

89630: eval: clean up usage of UnwrapDatum r=yuzefovich a=yuzefovich

This commit removes many calls to `eval.UnwrapDatum` where the first argument was `nil`. In such a case that function is equivalent to `tree.UnwrapDOidWrapper`, so this commit uses the latter wherever possible.

Next, this commit adds an explicit `context.Context` argument to the signature to avoid the usage of the deprecated stored context from `eval.Context`. This required a little bit of plumbing around the index encoding methods.

Release note: None

Epic: None

89648: server: allow configuring vmodule via cluster setting r=dt a=dt

Closes #89298.

Release note: none.

Epic: none.

90659: backfill: retain dropping columns when validating inverted index r=Xiang-Gu a=Xiang-Gu

Previously, when validating a forward index, we made the first mutation(s) public with two filters: ignore constraints and retain dropping columns. But we forgot to include the retain-dropping-columns policy when validating *inverted* index. This will only manifest itself in rare cases involving dropping column and validating inverted indexes. An example to trigger this rare case is:
```
create table t (j int[], k int not null, inverted index (j));

alter table t alter primary key using columns (k);
```

Fixes #90306
Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Andrew Baptist <[email protected]>
Co-authored-by: David Taylor <[email protected]>
Co-authored-by: Xiang Gu <[email protected]>
@craig craig bot closed this as completed in 78188e6 Oct 26, 2022
nicktrav pushed a commit to nicktrav/cockroach that referenced this issue Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli-server CLI commands that pertain to CockroachDB server processes A-observability-inf C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-server-and-security DB Server & Security
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant