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

kvserver: alias range_id in flow control integration tests #138085

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kvoli
Copy link
Collaborator

@kvoli kvoli commented Dec 30, 2024

Earlier commits are mechanical, reformatting and deduplicating SQL queries
used by the flow control integration tests.


Previously, the range_id of scratch ranges used in various
TestFlowControl.* tests would be printed into the testdata file. This
made it tedious to introduce new system tables, which in turn may create
new ranges and necessitate regenerating the
test_flow_control_integration testdata files to pick up the higher
default scratch range id.

Alias the range id to letters in a,b,...,z so that regardless of the
underlying scratch range id changing, the aliased scratch range id
remains unchanged. For example:

An old example:

SELECT
  range_id, count(*) AS streams
FROM
  crdb_internal.kv_flow_control_handles
GROUP BY
  (range_id)
ORDER BY
  streams DESC;

  range_id | stream_count
-----------+---------------
  75       | 3
  76       | 3

Now looks like:

SELECT
  chr(96 + DENSE_RANK() OVER (ORDER BY range_id)) as range_id,
  count(*) AS streams
FROM
  crdb_internal.kv_flow_control_handles
GROUP BY
  range_id
ORDER BY
  range_id;

  range_id | stream_count
-----------+---------------
  a        | 3
  b        | 3

Resolves: #137727
Release note: None

These were duplicated around many tests in
`flow_control_integration_test.go`. De-duplicate.

Part of: cockroachdb#137727
Release note: None
The SQL query strings which are reused across several
`TestFlowControl.*` tests were inconsistently formatted. Format them
consistently, although not following any style guide.

Part of: cockroachdb#137727
Release note: None
@kvoli kvoli self-assigned this Dec 30, 2024
Copy link

blathers-crl bot commented Dec 30, 2024

Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Previously, the `range_id` of scratch ranges used in various
`TestFlowControl.*` tests would be printed into the testdata file. This
made it tedious to introduce new system tables, which in turn may create
new ranges and necessitate regenerating the
`test_flow_control_integration` testdata files to pick up the higher
default scratch range id.

Alias the range id to letters in `a,b,...,z` so that regardless of the
underlying scratch range id changing, the aliased scratch range id
remains unchanged. For example:

An old example:

```sql
SELECT
  range_id, count(*) AS streams
FROM
  crdb_internal.kv_flow_control_handles
GROUP BY
  (range_id)
ORDER BY
  streams DESC;

  range_id | stream_count
-----------+---------------
  75       | 3
  76       | 3
```

Now looks like:

```sql
SELECT
  chr(96 + dense_rank() OVER (ORDER BY range_id)) as range_id,
  count(*) AS streams
FROM
  crdb_internal.kv_flow_control_handles
GROUP BY
  range_id
ORDER BY
  range_id;

  range_id | stream_count
-----------+---------------
  a        | 3
  b        | 3
```

Resolves: cockroachdb#137727
Release note: None
@kvoli kvoli force-pushed the 241230.rac-stop-printing-range-id branch from 5e14604 to 6975c56 Compare December 30, 2024 18:01
@kvoli kvoli requested a review from sumeerbhola January 2, 2025 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kvserver: stop printing range ids in flow control integration tests
2 participants