Merge #151147 #151180

151147: CLAUDE: add details and adjust directions for dev workflow r=rafiss a=rafiss

- Linting takes a while, so stop recommending it before every commit.
- Add information on optimizer and schemachanger packages.
- Mention bazel file generation.

Epic: None
Release note: None

151180: security: address a couple of nits in a recent change r=yuzefovich a=yuzefovich

I don't see why we would make the cluster setting a SystemVisible given that it affects every tenant and seems ok to be allowed for a tenant to change. Also there was a typo in the setting name as well as we should've registered the setting as ByteSize (which gives non-negative int validation). Additionally, unify the logging channels used for the error message (both will now go to DEV).

Touches: #151041.

Epic: None
Release note: None

Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>

Author: 3 people

CLAUDE.md

@@ -29,7 +29,7 @@ CockroachDB is a distributed SQL database written in Go, built with Bazel and ma
 ./dev test pkg/sql --stress          # Run repeatedly until failure
 ./dev testlogic                      # Run all SQL logic tests
 ./dev testlogic ccl                  # Run enterprise logic tests
-./dev testlogic --files='prepare|fk' # Run specific test files
+./dev testlogic base --config=local --files='prepare|fk' # Run specific test files under a specific configuration
 ```
 
 Note that when filtering tests via `-f` to include the `-v` flag which
@@ -40,8 +40,10 @@ for `testing: warning: no tests to run` in the output.
 ```bash
 ./dev generate            # Generate all code (protobuf, parsers, etc.)
 ./dev generate go         # Generate Go code only
-./dev lint                # Run all linters
-./dev lint --short        # Run fast subset of linters
+./dev generate bazel      # Update BUILD.bazel files when dependencies change
+./dev generate protobuf   # Generate files based on protocol buffer definitions
+./dev lint                # Run all linters (only run this when requested)
+./dev lint --short        # Run fast subset of linters (only run this when requested)
 ```
 
 ### Architecture Overview
@@ -67,20 +69,21 @@ SQL Layer (pkg/sql/) → Distributed KV (pkg/kv/) → Storage (pkg/storage/)
 
 ### Development Workflow
 
-1. **Environment Setup**: Run `./dev doctor` to ensure all dependencies are installed
-2. **Building**: Use `./dev build short` for iterative development, `./dev build cockroach` for full builds
-3. **Testing**: Run package-specific tests with `./dev test pkg/[package]` 
-4. **Code Generation**: After schema/proto changes, run `./dev generate go`
-5. **Quality Checks**: Run `./dev lint` before committing
+1. **Environment Setup**: Run `./dev doctor` to ensure all dependencies are installed.
+2. **Building**: Use `./dev build short` for iterative development, `./dev build cockroach` for full builds.
+3. **Testing**: Run package-specific tests with `./dev test pkg/[package]`.
+4. **Code Generation**: After schema/proto changes, run `./dev generate go`.
+5. **Linting**: Run with `./dev lint` or `./dev lint --short`. This takes a while, so no need to run it regularly.
 
 ### Testing Strategy
 
 CockroachDB has comprehensive testing infrastructure:
-- **Unit Tests**: Standard Go tests throughout `/pkg/` packages
-- **Logic Tests**: SQL correctness tests using `./dev testlogic`
-- **Roachtests**: Distributed system integration tests
-- **Acceptance Tests**: End-to-end testing in `/pkg/acceptance/`
-- **Stress Testing**: Continuous testing with `--stress` flag
+- **Unit Tests**: Standard Go tests throughout `/pkg/` packages.
+- **Logic Tests**: SQL correctness tests using `./dev testlogic`.
+- **Roachtests**: Distributed system integration tests.
+- **Acceptance Tests**: End-to-end testing in `/pkg/acceptance/`.
+- **Stress Testing**: Continuous testing with `--stress` flag.
+
 
 ### Build System
 
@@ -93,6 +96,8 @@ CockroachDB has comprehensive testing infrastructure:
 
 **Package Structure:**
 - `/pkg/sql/` - SQL layer (parser, optimizer, executor)
+- `/pkg/sql/opt` - Query optimizer and planner
+- `/pkg/sql/schemachanger` - Declarative schema changer
 - `/pkg/kv/` - Key-value layer and transaction management
 - `/pkg/storage/` - Storage engine interface
 - `/pkg/server/` - Node and cluster management

pkg/security/certificate_manager.go

@@ -198,9 +198,9 @@ func (cm *CertificateManager) RegisterSignalHandler(
 	})
 }
 
-var CertCacheMemLimit = settings.RegisterIntSetting(
-	settings.SystemVisible,
-	"security.clienc_cert.cache_memory_limit",
+var certCacheMemLimit = settings.RegisterByteSizeSetting(
+	settings.ApplicationLevel,
+	"security.client_cert.cache_memory_limit",
 	"memory limit for the client certificate expiration cache",
 	1<<29, // 512MiB
 )
@@ -214,7 +214,7 @@ func (cm *CertificateManager) RegisterExpirationCache(
 	parentMon *mon.BytesMonitor,
 	st *cluster.Settings,
 ) error {
-	limit := CertCacheMemLimit.Get(&st.SV)
+	limit := certCacheMemLimit.Get(&st.SV)
 	m := mon.NewMonitorInheritWithLimit(mon.MakeName("client-expiration-caches"), limit, parentMon, true /* longLiving */)
 	acc := m.MakeConcurrentBoundAccount()
 	m.StartNoReserved(ctx, parentMon)

pkg/security/clientcert/cert_expiry_cache.go

@@ -99,7 +99,7 @@ func (c *Cache) Upsert(ctx context.Context, user string, serial string, newExpir
 	if _, ok := c.cache[user]; !ok {
 		err := c.account.Grow(ctx, 2*GaugeSize)
 		if err != nil {
-			log.Ops.Warningf(ctx, "no memory available to cache cert expiry: %v", err)
+			log.Warningf(ctx, "no memory available to cache cert expiry: %v", err)
 			return
 		}
 		c.cache[user] = map[string]certInfo{}