Merge #157030 #158631 #158796 #158998

157030: sql: alter column identity with the declarative schema changer r=bghal a=bghal

Adds support for the `ALTER COLUMN ... [RESTART | SET [CACHE | CYCLE |
... ]` identity operations in the declarative schema changer.

Fixes:#142914
Epic: CRDB-31283

Release note (sql change): The `ALTER COLUMN ...` sequence identity
commands are run by the declarative schema changer.

158631: jobs: hold claim on failed job for one adoption cycle r=dt,stevendanna a=kev-cao

This commit teaches the job system to hold the claim on failed jobs for one adoption cycle before releasing the claim. This alleviates hot loops where a fast-failing job can be picked up repeatedly by different nodes within an adoption cycle.

Fixes: #158597

Release note: None

158796: logictest: reduce test churn when adding new system table r=yuzefovich a=yuzefovich

This commit reduces the test churn when adding a new system table via:
- eliding rangeID prefix from 'sending batch ...' trace message
- replacing the tableIDs in messages like '<before:/Table/77>' and '<after:/Table/107/1>' with 'XX'.

Additionally it adjusts a handful of tests to avoid requesting the range ID where it didn't seem required for the purpose of the test.

It also removes duplicated query from `information_schema` test which should've been removed when `local-mixed-23.2` config was removed.

Addresses: #135989.
Epic: None
Release note: None

158998: .claude: add commit-helper and backport-pr-assistant skills according to CockroachDB conventions r=rafiss a=rafiss

### .claude: add commit-helper skill for CockroachDB conventions

Add a Claude Code skill that assists with creating properly formatted
commit messages and release notes following CockroachDB conventions.
The skill encodes the project's commit message structure, release note
categories, and best practices to help ensure consistency across
contributions.

The skill covers:
- Commit message structure (package prefix, imperative mood, body format)
- Release note categories and when to include/exclude them
- Issue and epic reference formatting
- Common mistakes to avoid

###  .claude: add backport-pr-assistant skill for conflict resolution

Add a Claude Code skill that assists with backporting PRs to release
branches when merge conflicts require manual resolution. The skill
provides reference documentation for the backport CLI tool and
guidelines for resolving conflicts during the backport process.

This is implemented as a skill (rather than an agent) because simple
backports are handled by automation, so Claude is only invoked for
cases with conflicts that require interactive discussion and
resolution.

The skill covers:
- Backport CLI tool usage and common commands
- Workflow for handling conflicts during backports
- Guidelines for simple vs complex conflict resolution
- Quality checks after successful backport

Epic: None
Release note: None

Co-authored-by: Brendan Gerrity <[email protected]>
Co-authored-by: Kevin Cao <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>

Author: 5 people

.claude/skills/backport-pr-assistant/SKILL.md

@@ -0,0 +1,66 @@
+---
+name: backport-pr-assistant
+description: Help backport PRs to release branches using the backport CLI tool. Use when backporting changes that have merge conflicts requiring manual resolution.
+---
+
+# CockroachDB Backport Assistant
+
+Help the user backport pull requests to older release branches, especially when conflicts need resolution.
+
+## Backport CLI Tool Reference
+
+**Basic Usage:**
+```bash
+backport <pull-request>...              # Backport entire PR(s)
+backport <pr> -r <release>              # Target specific release (e.g., -r 23.2)
+backport <pr> -b <branch>               # Target specific branch (e.g., -b release-23.1.10-rc)
+backport <pr> -j "justification"        # Add release justification
+backport <pr> -c <commit> -c <commit>   # Cherry-pick specific commits only
+backport <pr> -f                        # Force operation
+```
+
+**Conflict Resolution:**
+```bash
+backport --continue                     # Resume after resolving conflicts
+backport --abort                        # Cancel in-progress backport
+```
+
+**Common Examples:**
+```bash
+backport 23437                          # Simple backport
+backport 23437 -r 23.2                  # To release-23.2 branch
+backport 23437 -j "test-only changes"   # With justification
+backport 23437 -b release-23.1.10-rc    # To specific release candidate branch
+```
+
+## Workflow
+
+1. **Start the backport**: Run `backport <pr> -r <release>` for the target branch
+2. **When conflicts occur**: The tool stops and lists conflicting files
+3. **Analyze conflicts**: Read the conflicting files, understand what's different between branches
+4. **Resolve conflicts**: Edit files to resolve, then `git add` the resolved files
+5. **Continue**: Run `backport --continue` to resume
+6. **Repeat** if more conflicts arise
+7. **Complete**: The backport tool pushes and creates the PR (do not use `gh` CLI to make the PR)
+
+## Conflict Resolution Guidelines
+
+**Simple conflicts you can resolve directly:**
+- Import statement conflicts
+- Simple variable name changes
+- Basic formatting differences
+- Minor API signature changes that are obvious
+
+**Complex conflicts - ask the user for guidance:**
+- Conflicts involving significant logic changes
+- Dependencies that don't exist in the target branch
+- API changes requiring substantial modification
+- Multiple conflicting files with interdependent changes
+- Changes that may not be appropriate for the target branch
+
+## When Resolving Conflicts
+
+1. **Explain what's conflicting** - show the relevant code sections
+2. **Explain why** - what's different between branches that caused this
+3. **Propose a resolution** - or ask for guidance if complex
+4. **After resolving**: `git add <files>` then `backport --continue`

.claude/skills/commit-helper/SKILL.md

@@ -0,0 +1,121 @@
+---
+name: commit-helper
+description: Help create git commits and PRs with properly formatted messages and release notes following CockroachDB conventions. Use when committing changes or creating pull requests.
+---
+
+# CockroachDB Commit Helper
+
+Help the user create properly formatted commit messages and release notes that follow CockroachDB conventions.
+
+## Workflow
+
+1. **Analyze the changes**: Run `git diff --staged` or `git diff` to understand what was modified
+2. **Determine the package prefix**: Identify the primary package affected
+3. **Ask the user** for:
+   - Issue/epic number if not already known
+   - Whether release notes are needed and what category fits best
+4. **Write the subject line**: Imperative mood, no period, under 72 characters
+5. **Write the body**: Explain the before/after, why the change was needed
+6. **Add issue references**: Include Resolves/Epic as appropriate
+7. **Write the release note**: Clear, user-focused description of the change
+8. **Create the commit** using the properly formatted message
+
+## Commit Message Structure
+
+**Basic Format:**
+```
+package: imperative title without period
+
+Detailed explanation of what changed, why it changed, and
+how it impacts users. Explain the problem that existed
+before and how this commit solves it.
+
+Include context about alternate approaches considered and
+any side effects or consequences.
+
+Resolves: #123
+Epic: CRDB-357
+
+Release note (category): Description of user-facing change
+in past or present tense explaining what changed, how users
+can see the change, and why it's important.
+```
+
+**Key Requirements:**
+- **Must** include release note annotation (even if "Release note: None")
+- **Must** include issue or epic reference
+- **Must** separate subject from body with blank line
+- **Recommended** prefix subject with affected package/area
+- **Recommended** use imperative mood in subject (e.g. "fix bug" not "fixes bug")
+- **Recommended** wrap body at 72-100 characters
+
+## Release Note Categories
+
+**When to include release notes:**
+- Changes to user interaction or experience
+- Changes to product behavior (performance, command responses, architecture)
+- Bug fixes affecting external users
+
+**When to exclude release notes:**
+- Internal refactors, testing, infrastructure work
+- Code that's too immature for docs (private preview features)
+- Internal settings beginning with `crdb_internal.`
+- Functionality not accessible to external users
+
+**Valid Categories:**
+- `backward-incompatible change` - Breaking changes to stable interfaces
+- `enterprise change` - Features requiring enterprise license
+- `ops change` - Commands/endpoints for operators (logging, metrics, CLI flags)
+- `cli change` - Commands for developers/contributors (SQL shells, debug tools)
+- `sql change` - SQL statements, functions, system catalogs
+- `ui change` - DB Console changes
+- `security update` - Security feature changes
+- `performance improvement` - Performance enhancements
+- `cluster virtualization` - Multi-tenancy infrastructure
+- `bug fix` - Problem fixes
+- `general change` - Changes that don't fit elsewhere
+- `build change` - Source build requirements
+
+## Release Note Best Practices
+
+**Description guidelines:**
+- Default to more information rather than less
+- Explain **what** changed, **how** it changed, and **why** it's relevant
+- Use past tense ("Added", "Fixed") or present tense ("CockroachDB now supports")
+- For bug fixes: describe cause, symptoms, and affected versions
+- Note if change is part of broader roadmap feature
+
+**Examples:**
+
+**Good bug fix:**
+```
+Release note (bug fix): Fixed a bug introduced in v19.2.3 that
+caused duplicate rows in CREATE TABLE ... AS results when multiple
+nodes attempt to populate the results.
+```
+
+**Good feature:**
+```
+Release note (enterprise change): Shortened the default interval
+for the kv.closed_timestamp.target_duration cluster setting from
+30s to 3s. This allows follower reads at 4.8 seconds in the past,
+a much shorter window than the previous 48 seconds.
+```
+
+## Issue References
+- `Resolves: #123` - Auto-closes issue on PR merge
+- `See also: #456, #789` - Cross-references issues
+- `Epic: CRDB-357` - Links to epic
+
+## How to Avoid Common Pitfalls
+- Always include a release note annotation (even "Release note: None")
+- Use only valid category names from the list above
+- Keep release notes focused on user-facing information
+- Write specific descriptions that explain the impact to users
+- Use `backward-incompatible change` category for any breaking changes
+- End subject lines without punctuation
+- Explain the "why" behind changes, not just the "what"
+
+## Pull Request Guidelines
+- **Create PRs from your personal fork**, not directly on cockroachdb/cockroach
+- **Single-commit PRs**: PR title should match commit title, PR body should match commit body

pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant

@@ -248,15 +248,15 @@ SELECT * FROM crdb_internal.node_txn_execution_insights WHERE query = ''
 ----
 txn_id  txn_fingerprint_id  query  implicit_txn  session_id  start_time  end_time  user_name  app_name  rows_read  rows_written  priority  retries  last_retry_reason  contention  problems  causes  stmt_execution_ids  cpu_sql_nanos  last_error_code  last_error_redactable status
 
-query ITTI
-SELECT range_id, start_pretty, end_pretty, lease_holder FROM crdb_internal.ranges
+query TTI
+SELECT start_pretty, end_pretty, lease_holder FROM crdb_internal.ranges
 ----
-80  /Tenant/10  /Tenant/11  1
+/Tenant/10  /Tenant/11  1
 
-query ITT
-SELECT range_id, start_pretty, end_pretty FROM crdb_internal.ranges_no_leases
+query TT
+SELECT start_pretty, end_pretty FROM crdb_internal.ranges_no_leases
 ----
-80  /Tenant/10  /Tenant/11
+/Tenant/10  /Tenant/11
 
 query IT
 SELECT zone_id, target FROM crdb_internal.zones ORDER BY 1

pkg/ccl/logictestccl/testdata/logic_test/multi_region_remote_access_error

@@ -468,10 +468,9 @@ messages_global
 
 # Non-DML SHOW RANGES statement on RBR table should succeed.
 skipif config multiregion-9node-3region-3azs-vec-off
-query I retry
+retry
+statement ok
 SELECT DISTINCT range_id FROM [SHOW RANGES FROM TABLE messages_rbr]
-----
-85
 
 # Update does not fail when accessing all rows in messages_rbr because lookup
 # join does not error out the lookup table in phase 1.

pkg/ccl/logictestccl/testdata/logic_test/regional_by_row_insert_fast_path

@@ -200,7 +200,7 @@ query T rowsort
 SELECT message FROM [SHOW KV TRACE FOR SESSION]
 WHERE message LIKE '%batch%' AND message LIKE '%Scan%'
 ----
-r79: sending batch 4 Scan to (n1,s1):1
+sending batch 4 Scan to (n1,s1):1
 
 # Regression test for #115377.
 statement ok

pkg/ccl/logictestccl/testdata/logic_test/regional_by_row_query_behavior

@@ -266,7 +266,7 @@ ap-southeast-2  23
 query TT
 SELECT start_key, end_key FROM [SHOW RANGE FROM TABLE regional_by_row_table FOR ROW ('ap-southeast-2', 1)]
 ----
-<before:/Table/76>  …
+<before:/Table/XX>  …
 
 query TIIII
 SELECT crdb_region, pk, pk2, a, b FROM regional_by_row_table
@@ -404,9 +404,9 @@ SELECT start_key, end_key, replicas, lease_holder FROM [SHOW RANGES FROM INDEX r
 ORDER BY 1
 ----
 start_key           end_key               replicas  lease_holder
-<before:/Table/76>  …/"\x80"/0            {1}       1
+<before:/Table/XX>  …/"\x80"/0            {1}       1
 …/"\x80"/0          …/"\xc0"/0            {4}       4
-…/"\xc0"/0          <after:/Table/110/5>  {7}       7
+…/"\xc0"/0          <after:/Table/XX/5>  {7}       7
 
 statement ok
 SET locality_optimized_partitioned_index_scan = false

pkg/cmd/roachtest/tests/mixed_version_job_compatibility_in_declarative_schema_changer.go

@@ -127,6 +127,7 @@ func executeSupportedDDLs(
 	v261DDLs := []string{
 		`ALTER TABLE testdb.testsc.t2 ALTER COLUMN j SET NOT VISIBLE`,
 		`ALTER TABLE testdb.testsc.t2 ALTER COLUMN j SET VISIBLE`,
+		`ALTER TABLE testdb.testsc.t2 ALTER COLUMN l SET MAXVALUE 40 RESTART WITH 20 SET CACHE 5 SET INCREMENT BY 2`,
 	}
 
 	// Used to clean up our CREATE-d elements after we are done with them.
@@ -209,7 +210,7 @@ CREATE DATABASE IF NOT EXISTS testdb;
 CREATE SCHEMA IF NOT EXISTS testdb.testsc;
 CREATE TABLE IF NOT EXISTS testdb.testsc.t (i INT PRIMARY KEY, j INT NOT NULL, INDEX idx (j), CONSTRAINT check_j CHECK (j > 0));
 INSERT INTO testdb.testsc.t VALUES (1, 1);
-CREATE TABLE IF NOT EXISTS testdb.testsc.t2 (i INT NOT NULL, j INT NOT NULL, k STRING NOT NULL);
+CREATE TABLE IF NOT EXISTS testdb.testsc.t2 (i INT NOT NULL, j INT NOT NULL, k STRING NOT NULL, l INT GENERATED ALWAYS AS IDENTITY);
 INSERT INTO testdb.testsc.t2 VALUES (2, 3, 'foo');
 CREATE TABLE IF NOT EXISTS testdb.testsc.t3 (i INT NOT NULL, j INT NOT NULL, k STRING NOT NULL);
 INSERT INTO testdb.testsc.t3 VALUES (3, 3, 'bar');

pkg/jobs/adopt.go

@@ -9,6 +9,7 @@ import (
 	"context"
 	"fmt"
 	"sync"
+	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
 	"github.com/cockroachdb/cockroach/pkg/kv"
@@ -423,14 +424,28 @@ func (r *Registry) runJob(
 	}
 
 	r.maybeRecordExecutionFailure(ctx, err, job)
-	// NB: After this point, the job may no longer have the claim
-	// and further updates to the job record from this node may
-	// fail.
-	r.maybeClearLease(job, err)
 	r.maybeDumpTrace(ctx, resumer, job.ID())
+
 	if r.knobs.AfterJobStateMachine != nil {
 		r.knobs.AfterJobStateMachine(job.ID())
 	}
+
+	// NB: After this point, the job may no longer have the claim and further
+	// updates to the job record from this node may fail.
+	if err != nil {
+		// We delay clearing the lease to avoid situations where a job fast-fails
+		// and is immediately re-adopted by another node, causing a hot loop of job
+		// status changes. See #158597 for more info.
+		//
+		// NB: The node that holds this claim also will not rerun the job until
+		// this interval elapses and the job is removed from the list of currently
+		// running jobs.
+		select {
+		case <-ctx.Done():
+		case <-time.After(r.GetLoopInterval(adoptIntervalSetting, r.knobs.IntervalOverrides.ClaimTTLOnFailure)):
+		}
+		r.maybeClearLease(job, err)
+	}
 	return err
 }
 

pkg/jobs/config.go

@@ -11,7 +11,6 @@ import (
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/settings"
-	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 )
 
@@ -177,18 +176,13 @@ type loopController struct {
 // at regular intervals. The structure's cleanup method should be deferred to
 // execute before destroying the instantiated structure.
 func makeLoopController(
-	st *cluster.Settings, s *settings.DurationSetting, overrideKnob *time.Duration,
+	r *Registry, s *settings.DurationSetting, overrideKnob *time.Duration,
 ) loopController {
 	lc := loopController{
 		lastRun: timeutil.Now(),
 		updated: make(chan struct{}, 1),
-		// getInterval returns the value of the associated cluster setting. If
-		// overrideKnob is not nil, it overrides the cluster setting.
 		getInterval: func() time.Duration {
-			if overrideKnob != nil {
-				return *overrideKnob
-			}
-			return time.Duration(intervalBaseSetting.Get(&st.SV) * float64(s.Get(&st.SV)))
+			return r.GetLoopInterval(s, overrideKnob)
 		},
 	}
 
@@ -202,8 +196,8 @@ func makeLoopController(
 	}
 
 	// register onChange() to get a notification when the cluster is updated.
-	s.SetOnChange(&st.SV, onChange)
-	intervalBaseSetting.SetOnChange(&st.SV, onChange)
+	s.SetOnChange(&r.settings.SV, onChange)
+	intervalBaseSetting.SetOnChange(&r.settings.SV, onChange)
 
 	lc.timer.Reset(jitter(lc.getInterval()))
 	return lc