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

kv/kvserver: TestBackpressureNotAppliedWhenReducingRangeSize failed #75109

Closed
cockroach-teamcity opened this issue Jan 18, 2022 · 1 comment · Fixed by #75233
Closed

kv/kvserver: TestBackpressureNotAppliedWhenReducingRangeSize failed #75109

cockroach-teamcity opened this issue Jan 18, 2022 · 1 comment · Fixed by #75233
Assignees
@irfansharif
Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. T-kv KV Team

Comments

@cockroach-teamcity
Copy link
Member

kv/kvserver.TestBackpressureNotAppliedWhenReducingRangeSize failed with artifacts on master @ 3b9a9579bed8288a7823d6caeebf692168987459:

=== RUN   TestBackpressureNotAppliedWhenReducingRangeSize
    test_log_scope.go:79: test logs captured to: /go/src/github.com/cockroachdb/cockroach/artifacts/logTestBackpressureNotAppliedWhenReducingRangeSize3351118793
    test_log_scope.go:80: use -show-logs to present logs inline
=== CONT  TestBackpressureNotAppliedWhenReducingRangeSize
    client_replica_backpressure_test.go:310: -- test log scope end --
test logs left over in: /go/src/github.com/cockroachdb/cockroach/artifacts/logTestBackpressureNotAppliedWhenReducingRangeSize3351118793
--- FAIL: TestBackpressureNotAppliedWhenReducingRangeSize (5.58s)
=== RUN   TestBackpressureNotAppliedWhenReducingRangeSize/backpressure_when_near_limit_on_new_node
    client_replica_backpressure_test.go:303: expected no error because the request should hang, got <nil>
    --- FAIL: TestBackpressureNotAppliedWhenReducingRangeSize/backpressure_when_near_limit_on_new_node (1.54s)
Help

See also: How To Investigate a Go Test Failure (internal)
Parameters in this failure:

  • GOFLAGS=-json

Same failure on other branches

/cc @cockroachdb/kv

This test on roachdash | Improve this report!
@cockroach-teamcity cockroach-teamcity added branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. labels Jan 18, 2022
@irfansharif irfansharif self-assigned this Jan 20, 2022
@irfansharif
Copy link
Contributor

Not the easiest thing to repro; on my GCE worker:

$ dev test pkg/kv/kvserver -f=TestBackpressureNotAppliedWhenReducingRangeSize/backpressure_when_near_limit_on_new_node -v --show-logs --timeout 5m --stress
[ ... ]
3649 runs completed, 0 failures, over 5m0s
SUCCESS
Target //pkg/kv/kvserver:kvserver_test up-to-date:
  _bazel/bin/pkg/kv/kvserver/kvserver_test_/kvserver_test
INFO: Elapsed time: 303.564s, Critical Path: 301.75s
INFO: 670 processes: 655 remote cache hit, 14 internal, 1 local.
INFO: Build completed successfully, 670 total actions
//pkg/kv/kvserver:kvserver_test                                          PASSED in 301.9s

@irfansharif irfansharif mentioned this issue Jan 20, 2022
Merged
@tbg tbg added the T-kv KV Team label Jan 24, 2022
craig bot pushed a commit that referenced this issue Jan 25, 2022
@arulajmani @irfansharif @stevendanna
Merge #74765 #75233 #75280
c956f4c 
74765: roachpb: introduce the concept of `SystemSpanConfig` and related protos r=arulajmani a=arulajmani

This patch is motivated by the desire to let the host tenant lay
protected timestamps on one or all secondary tenants' keyspace. It
also provides a mechanism to allow secondary tenants to lay protected
timestamps on their entire keyspace without updating every span
configuration.

We introduce the concept of `SystemSpanConfig` and
`SystemSpanConfigTarget` to enable this. We tie these together using a
`SystemSpanConfigEntry`.

A `SystemSpanConfig` is a system installed configuration that can apply
to multiple spans. It only contains protected timestamp information.

A `SystemSpanConfigTarget` is used to specify the spans a
`SystemSpanConfig` applies over. It can be used to target the entire
(logical) cluster or a particular secondary tenant. We will ensure that
only the host tenant can target secondary tenants in a future PR that
actually persists `SystemSpanConfigs`.

We will persist `SystemSpanConfigs` in `system.span_configurations` in
a future patch. The `SystemSpanConfigTarget` will be encoded into
special reserved keys when we do so.

This change introduces the notion of a hierarchy to span configurations.
The configuration that applies to a span will now bee the `SpanConfig`
stored in `system.span_configurations` combined with all the
`SystemSpanConfigs` that apply to the span. This can be at most 4
levels deep -- for a secondary tenant's range, the secondary tenant can
install a `SystemSpanConfig` that applies to all its ranges, the host
tenant can install a `SystemSpanConfig` that applies to all ranges of
the secondary tenant, and the host tenant can install a
`SystemSpanConfig` that applies to all ranges.

These protos form the data model which will later be used to enable
protected timestamp support for secondary tenants using the span config
infrastructure. It will be used by the various components such as the
`SQLTranslator`, `KVAccessor`, `Reconciler` etc.

Release note: None

75233: kvserver: avoid clobbering replica conf r=irfansharif a=irfansharif

Fixes #75109. There are two ways to read the configuration applied over
a given replica:
  (a) the copy embedded in each replica struct
  (b) spanconfig.StoreReader, hanging off the store struct

The interface in (b) is implemented by both the span configs
infrastructure and the legacy system config span it's designed to
replace; it's typically used by KV queues (see #69172). When switching
between subsystems in KV (controlled by spanconfig.store.enabled), for
we transparently switch the source for (b). We also use then use the
right source to refresh (a) for every replica. Incremental updates
thereafter mutate (a) directly. As you'd expect, we need to take special
care that only one subsystem is writing to (a) at a given point in time,
a guarantee that was not upheld before this commit. This bug manifested
after we enabled span configs by default (see #73876), and likely
affected many tests.

To avoid the system config span from clobbering (a) when span configs
are enabled, we just need to check what spanconfig.store.enabled
holds at the right spot. To repro:

    # Reliably fails with 1-2m on my GCE worker before this patch,
    # doesn't after.
    dev test pkg/kv/kvserver \
      -f TestBackpressureNotAppliedWhenReducingRangeSize \
      -v --show-logs --timeout 15m --stress

Release note: None

75280: sql: deprecate TableDescriptor.GCMutations r=postamar,ajwerner a=stevendanna

This appears unused. While the schema changer adds entries that the gc
job subsequently removes, the only other code that made use of this
field (outside of tests) was FindIndexByID. FindIndexByID appears to
use it to return a special error that no one looks for.

Release note: None

Co-authored-by: arulajmani <arulajmani@gmail.com>
Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>
Co-authored-by: Steven Danna <danna@cockroachlabs.com>
@craig craig bot closed this as completed in 0a6897d Jan 25, 2022
@irfansharif irfansharif mentioned this issue Jan 28, 2022
Closed
23 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@irfansharif irfansharif

Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. T-kv KV Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

kvserver: avoid clobbering replica conf irfansharif/cockroach
3 participants
@tbg @irfansharif @cockroach-teamcity