fix: disambiguate cluster_id <-> instance_id #2472
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR disambiguates between cluster_id and instance_id across the Rivet codebase, clarifying that instance_id uniquely identifies the entire Rivet cluster while cluster_id refers to specific clusters within an instance.
- Renamed
cluster_idcolumn torivet_instance_idinpackages/core/services/dynamic-config/db/dynamic-config/migrations/20250523120501_instance_id.up.sql - Added
instance_idfield to Rivet configuration inpackages/common/config/src/config/server/rivet/mod.rs - Updated telemetry beacon in
packages/core/services/telemetry/standalone/beacon/src/lib.rsto use instance_id for tracking - Modified
get_config.rsto handle instance_id with clear documentation about its purpose - Added instance_id support in worker and guard configurations for consistent identification
6 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile
| ALTER TABLE config | ||
| RENAME COLUMN cluster_id TO rivet_instance_id; |
There was a problem hiding this comment.
logic: This migration will break get_config.rs which still references the cluster_id column in its SQL queries. Update all dependent code before running this migration.
| /// If specified, will use this as the default cluster ID. | ||
| /// | ||
| /// This will have no effect if applied after the cluster has first ran. | ||
| pub instance_id: Option<Uuid>, |
There was a problem hiding this comment.
logic: Documentation comment appears to be incorrect - refers to 'default cluster ID' but this is for instance_id
| @@ -171,7 +177,7 @@ impl Rivet { | |||
| } | |||
| } | |||
|
|
|||
| impl Rivet { | |||
| impl Rivet { | |||
There was a problem hiding this comment.
syntax: Extra tab character after impl Rivet
| // Pick an instance ID to insert if none exists. If this is specified in the config. fall back to | ||
| // this. |
There was a problem hiding this comment.
syntax: Comment has a period in the middle of a sentence: 'If this is specified in the config. fall back to'
| // Pick an instance ID to insert if none exists. If this is specified in the config. fall back to | |
| // this. | |
| // Pick an instance ID to insert if none exists. If this is specified in the config, fall back to | |
| // this. |
| // /// This is helpful for diagnosing issues with the self-hosted instances under | ||
| // /// load. e.g. if a instance is running on constraint resources (see os_report), | ||
| // /// does the instance configuration affect it? |
There was a problem hiding this comment.
syntax: typo: 'constraint resources' should be 'constrained resources'
| // /// This is helpful for diagnosing issues with the self-hosted instances under | |
| // /// load. e.g. if a instance is running on constraint resources (see os_report), | |
| // /// does the instance configuration affect it? | |
| // /// This is helpful for diagnosing issues with the self-hosted instances under | |
| // /// load. e.g. if a instance is running on constrained resources (see os_report), | |
| // /// does the instance configuration affect it? |
Deploying rivet with
|
| Latest commit: |
08baa4f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9b743418.rivet.pages.dev |
| Branch Preview URL: | https://graphite-base-2473.rivet.pages.dev |
Merge activity
|
<!-- Please make sure there is an issue that this PR is correlated to. --> ## Changes <!-- If there are frontend changes, please include screenshots. -->
Changes