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

cdc: SHOW CHANGEFEED JOBS only shows jobs from the last 12h #97883

Closed
jayshrivastava opened this issue Mar 1, 2023 · 1 comment · Fixed by #98175
Closed

cdc: SHOW CHANGEFEED JOBS only shows jobs from the last 12h #97883

jayshrivastava opened this issue Mar 1, 2023 · 1 comment · Fixed by #98175
Assignees
Labels
A-cdc Change Data Capture C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-cdc

Comments

@jayshrivastava
Copy link
Contributor

jayshrivastava commented Mar 1, 2023

I got this escalation where a customer did not notice failed changefeed jobs because they did not show up in the SHOW CHANGEFEED JOBS output. The reason is that SHOW CHANGEFEED JOBS only shows running jobs and jobs from the last 12h.

We should make this show all changefeed jobs. If the customer does not care about ALL the jobs they can still filter on the output columns (ex. filter out cancelled jobs or jobs which finished/started some interval of time in the past).

Jira issue: CRDB-24944

Epic CRDB-11783

@jayshrivastava jayshrivastava added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-cdc Change Data Capture T-cdc labels Mar 1, 2023
@blathers-crl
Copy link

blathers-crl bot commented Mar 1, 2023

cc @cockroachdb/cdc

@jayshrivastava jayshrivastava self-assigned this Mar 6, 2023
jayshrivastava added a commit to jayshrivastava/cockroach that referenced this issue Mar 7, 2023
Release note (general change): Previously, the output of `SHOW CHANGEFEED JOBS`
was limited to show unfinished jobs and finished jobs from the last 14 days.
This change makes the command show all changefeed jobs, regardless of if they finished
and when they finished. Note that jobs still obey the cluster setting `jobs.retention_time`.
Completed jobs older than that time are deleted.

Fixes: cockroachdb#97883
Epic: None
craig bot pushed a commit that referenced this issue Mar 14, 2023
95789: pkg/util/log: don't falsify tenant ID tag in logs if none in ctx r=andreimatei a=abarganier

Previously, I made the decision to always tag a log entry with a tenant ID, even if no tenant ID was found in the context associated with the log entry. In this case, the system tenant ID was used in the tag, instead of omitting a tenant ID tag altogether.

I received some feedback that this is confusing. For example, imagine testing a feature, expecting log entries to come from a secondary tenant, and the context being used in that feature is not annotated with a tenant ID. With the previous behavior, the log entry would default to being tagged with the system tenant ID instead of having empty tags (or at least, no tenant ID tag). In this scenario, how do I tell the actual state of the log entry? Did the log entry indeed come from a goroutine belonging to the system tenant? Or was the context just missing the tenant ID annotation, but otherwise came from the correct tenant?

This ambiguity is not helpful. By falsifying a tenant ID tag we confuse the log reader about the actual state of the system. Furthermore, our eventual goal should be that almost no context objects in the system exist without a tenant ID (except for perhaps at startup before tenant initialization). Tagging with the system tenant ID in the case of a missing tenant ID annotation in the context makes it difficult to track down offending context objects.

This patch removes this default behavior from the logging package. Now, if no tenant ID is found in the context, we do not tag the entry with a tenant ID. Note however that on the *decode* side, we will maintain this default tenant ID tagging behavior. If a log entry does not have a tenant ID tag, then we must assume that only the system tenant has privilege to view said log entry, since the owner is ambiguous.

Release note: none

Epic CRDB-14486

98175: cdc: show all changefeed jobs in `SHOW CHANGEFEED JOBS` r=HonoreDB a=jayshrivastava

### cdc: show all changefeed jobs in SHOW CHANGEFEED JOBS

Release note (general change): Previously, the output of `SHOW CHANGEFEED JOBS` was limited to show unfinished jobs and finished jobs from the last 14 days. This change makes the command show all changefeed jobs, regardless of if they finished and when they finished. Note that jobs still obey the cluster setting `jobs.retention_time`. Completed jobs older than that time are deleted.

Fixes: #97883

### jobs: add virtual index for job_type in crdb_internal.jobs

This change adds a virtual index on the `job_type` column
of `crdb_internal.jobs`. This change should make queries
on that table which filter on job type (such as `SHOW
CHANGEFEED JOBS`) more efficient.

Release note: None

Epic: None

98515: kvserver: deflake test store capacity after split r=andrewbaptist a=kvoli

This commit defales `TestStoreCapacityAfterSplit`. Previously it was possible for the replica load stats which underpins Capacity to be reset. The reset caused the recording duration to fall short of min stats duration, which led to a 0 value being reported for writes in store capacity.

This commit bumps the manual clock twice and removes redundant leaseholder checks within a retry loop. The combination of these two changes makes the test much less likely to flake.

The test is now unskipped.

```
dev test pkg/kv/kvserver -f TestStoreCapacityAfterSplit -v --stress
...
4410 runs so far, 0 failures, over 6m10s
```

Resolves: #92677

Release note: None

98521: ui: don't continue polling endpoints that return 403 errors r=dhartunian a=abarganier

It was brought to our attention that endpoints such as `v1/settings` would continue to be polled by DB Console even if they returned 403 errors.

If an endpoint returns 403 errors, we should not continue to poll it since the required access is not present for the current user.

This patch updates the polling mechanism to short-circuit the `refresh` process if a 403 error is encountered throughout the lifecycle of the poller.

Release note: none

Fixes: #98356

98536: kvserver: deflake learner joint cfg relocate range r=andrewbaptist a=kvoli

Previously, in `TestLearnerOrJointConfigAdminRelocateRange` it was possible for there to be an in-flight snapshot towards a learner, prior to sending `AdminRelocateRange` command. When this occurred, the test would fail as `AdminRelocateRange` returns an error when finding any in-flight snapshots to learners. This situation occurred infrequently, causing the test to flake.

This commit updates the `TestLearnerOrJointConfigAdminRelocateRange` test to first assert that there are the expected number of learners, then assert that there are no in-flight snapshots towards learners before beginning the main testing logic. The test is now unskipped.

```
dev test pkg/kv/kvserver \
      -f TestLearnerOrJointConfigAdminRelocateRange \
      -v --stress
...
5652 runs so far, 0 failures, over 12m30s
```

Resolves: #95500

Release note: None

98542: storage: remove MVCCIterator.Key method r=jbowens a=jbowens

The MVCCIterator interface previously exposed two methods for accessing the current iterator postion as a MVCC key—UnsafeKey and Key. Key() was equivalent to UnsafeKey().Clone().

This commit removes the Key() variant, pushing the onus of key copying onto the caller. This reduces the interface surface area, avoids accidental key copying (some of which is addressed within this commit), and does not impose any unreasonable burden on callers.

Epic: None
Informs #82589.
Release note: None

98543: allocator: fix lease io enforcement setting typo r=andrewbaptist a=kvoli

This commit updates the "do nothing" lease IO overload enforcement (`kv.allocator.lease_io_overload_threshold_enforcement`) setting to be correctly spelled "ignore" rather than "ingore".

Part of: #96508

Release note (ops change): The
`kv.allocator.lease_io_overload_threshold_enforcement` setting value which disables enforcement is updated to be spelled correctly as "ignore" rather than "ingore".

98600: server: change conn close error to warning r=knz,abarganier a=dhartunian

Resolves: #98523
Epic: None
Release note: None

Co-authored-by: Alex Barganier <[email protected]>
Co-authored-by: Jayant Shrivastava <[email protected]>
Co-authored-by: Austen McClernon <[email protected]>
Co-authored-by: Jackson Owens <[email protected]>
Co-authored-by: David Hartunian <[email protected]>
@craig craig bot closed this as completed in 4fecf02 Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cdc Change Data Capture C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-cdc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant