admission: goschedstats sampling period is too long #111125

sumeerbhola · 2023-09-22T18:00:07Z

goschedstats uses a 250ms sampling interval when the CPU utilization is "low"

cockroach/pkg/util/goschedstats/runnable.go

Lines 216 to 220 in ed75b08

    
           if avgValue < underloadedRunnablePerProcThreshold && 
        
           	s.localEWMA < underloadedRunnablePerProcThreshold { 
        
           	// Underloaded, so switch to longer sampling period. 
        
           	nextPeriod = samplePeriodLong 
        
           }

This "low" is based on 1 runnable goroutine per proc
https://github.com/cockroachdb/cockroach/blob/master/pkg/util/goschedstats/runnable.go#L72

This threshold is too high.
Here is a node running at close to 80% cpu utilization but is mostly doing sampling at a 250ms interval, due to which the slot adjustment is not fast enough, and we see slot exhaustion.

Jira issue: CRDB-31779

111441: goschedstats: reduce underloadedRunnablePerProcThreshold r=aadityasondhi a=sumeerbhola By reducing this threshold, we more often sample the stats at 1ms intervals, which is desirable for admission control slot adjustment. Fixes #111125 Epic: none Release note: None 111521: roachprod: fix createTenantCertBundle script r=andy-kimball a=herkolategan The `createTenantCertBundle` function has a script in it that has erroneous "+" characters which results in an error when trying to start a secure cluster. This change removes those characters. Epic: None Release Note: None Co-authored-by: sumeerbhola <[email protected]> Co-authored-by: Herko Lategan <[email protected]>

109638: sql: use the correct locking strength for FOR SHARE clause r=michae2 a=arulajmani Previously, FOR SHARE and FOR KEY SHARE would use non-locking KV scans. Now that the lock table supports shared locks, we can use lock.Shared as the locking strength for KV scans. This patch does that, and in doing so, wires up SHARED locks end to end. By default, we turn of this functionality for serializable transactions. Instead, it's gated behind a session setting called `enable_shared_locking_for_serializable`. Informs #91545 Release note (sql change): SELECT FOR SHARE and SELECT FOR UPDATE previously did not acquire any locks. Users issuing these statements would expect them to acquire shared locks (multiple readers allowed, no writers though). This patch switches over the behavior to acquire such read locks. For serializable transactions, we default to the old behaviour, unless the `enable_shared_locking_for_serializable` session setting is set to true. We'll probably switch this behavior in the future, but for now, given shared locks are a preview feature, we gate things behind a session setting. 111441: goschedstats: reduce underloadedRunnablePerProcThreshold r=aadityasondhi a=sumeerbhola By reducing this threshold, we more often sample the stats at 1ms intervals, which is desirable for admission control slot adjustment. Fixes #111125 Epic: none Release note: None 111466: ui: update default timescale to 1h r=maryliag a=maryliag On the Metrics page, the default value was 10min, which was too small. This commit updates it to 1h to match the most selected value on the SQL Activity. Fixes #96479 Release note: None 111521: roachprod: fix createTenantCertBundle script r=andy-kimball a=herkolategan The `createTenantCertBundle` function has a script in it that has erroneous "+" characters which results in an error when trying to start a secure cluster. This change removes those characters. Epic: None Release Note: None 111526: kv/spanset: permit writes to lock table by shared lock latching configuration r=nvanbenschoten a=nvanbenschoten Fixes #111409. Fixes #111492. This commit updates addLockTableSpans to account for the way that shared locking requests declare their latches. Even though they declare a "read" latch, they do so at max timestamp, which gives them sufficient isolation to write to the lock table without having to declare a write latch and be serialized with other shared lock acquisitions. Release note: None Co-authored-by: Arul Ajmani <[email protected]> Co-authored-by: sumeerbhola <[email protected]> Co-authored-by: maryliag <[email protected]> Co-authored-by: Herko Lategan <[email protected]> Co-authored-by: Nathan VanBenschoten <[email protected]>

By reducing this threshold, we more often sample the stats at 1ms intervals, which is desirable for admission control slot adjustment. Fixes cockroachdb#111125 Epic: none Release note: None

sumeerbhola added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-admission-control labels Sep 22, 2023

sumeerbhola mentioned this issue Sep 28, 2023

goschedstats: reduce underloadedRunnablePerProcThreshold #111441

Merged

craig bot closed this as completed in 32adda2 Sep 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

admission: goschedstats sampling period is too long #111125

admission: goschedstats sampling period is too long #111125

sumeerbhola commented Sep 22, 2023 •

edited by cockroach-jira-scripts

Loading

admission: goschedstats sampling period is too long #111125

admission: goschedstats sampling period is too long #111125

Comments

sumeerbhola commented Sep 22, 2023 • edited by cockroach-jira-scripts Loading

sumeerbhola commented Sep 22, 2023 •

edited by cockroach-jira-scripts

Loading