-
Couldn't load subscription status.
- Fork 130
fix: disambiguate cluster_id <-> instance_id #2472
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,6 +60,11 @@ pub struct Rivet { | |
| #[serde(default = "Rivet::default_namespace")] | ||
| pub namespace: String, | ||
|
|
||
| /// 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>, | ||
|
|
||
| /// If specified, will use this as the default cluster ID. | ||
| /// | ||
| /// This will have no effect if applied after the cluster has first ran. | ||
|
|
@@ -139,6 +144,7 @@ impl Default for Rivet { | |
| fn default() -> Rivet { | ||
| Self { | ||
| namespace: Self::default_namespace(), | ||
| instance_id: None, | ||
| default_cluster_id: None, | ||
| clusters: None, | ||
| provision: None, | ||
|
|
@@ -171,7 +177,7 @@ impl Rivet { | |
| } | ||
| } | ||
|
|
||
| impl Rivet { | ||
| impl Rivet { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. syntax: Extra tab character after impl Rivet |
||
| pub fn default_cluster_id(&self) -> GlobalResult<Uuid> { | ||
| if let Some(default_cluster_id) = self.default_cluster_id { | ||
| ensure!( | ||
|
|
@@ -184,9 +190,7 @@ impl Rivet { | |
| // Return default development clusters | ||
| AccessKind::Development => Ok(default_dev_cluster::CLUSTER_ID), | ||
| // No cluster configured | ||
| AccessKind::Public | AccessKind::Private => { | ||
| bail!("`default_cluster_id` not configured") | ||
| } | ||
| AccessKind::Public | AccessKind::Private => bail!("`default_cluster_id` not configured"), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| ALTER TABLE config | ||
| RENAME COLUMN cluster_id TO rivet_instance_id; | ||
|
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,58 +1,58 @@ | ||||||||||
| use chirp_workflow::prelude::*; | ||||||||||
| use tokio::sync::OnceCell; | ||||||||||
|
|
||||||||||
| // The cluster ID will never change, so store it in memory. | ||||||||||
| static CLUSTER_ID_ONCE: OnceCell<Uuid> = OnceCell::const_new(); | ||||||||||
| // The instance ID will never change, so store it in memory. | ||||||||||
| static INSTANCE_ID_ONCE: OnceCell<Uuid> = OnceCell::const_new(); | ||||||||||
|
|
||||||||||
| #[derive(Debug, Default)] | ||||||||||
| pub struct Input {} | ||||||||||
|
|
||||||||||
| #[derive(Debug)] | ||||||||||
| pub struct Output { | ||||||||||
| pub cluster_id: Uuid, | ||||||||||
| pub instance_id: Uuid, | ||||||||||
| } | ||||||||||
|
|
||||||||||
| #[operation] | ||||||||||
| pub async fn get_cluster_id(ctx: &OperationCtx, input: &Input) -> GlobalResult<Output> { | ||||||||||
| pub async fn get_config(ctx: &OperationCtx, input: &Input) -> GlobalResult<Output> { | ||||||||||
| // IMPORTANT: This is not the same as the cluster ID from the `cluster` package. This is used | ||||||||||
| // for unqiuely identifying the entire Rivet cluster. | ||||||||||
| // for uniquely identifying the entire Rivet cluster. | ||||||||||
|
|
||||||||||
| // Pick a cluster ID to insert if none exists. 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. | ||||||||||
|
Comment on lines
+20
to
21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. syntax: Comment has a period in the middle of a sentence: 'If this is specified in the config. fall back to'
Suggested change
|
||||||||||
| let default_cluster_id = | ||||||||||
| if let Some(cluster_id) = ctx.config().server()?.rivet.default_cluster_id { | ||||||||||
| cluster_id | ||||||||||
| let default_instance_id = | ||||||||||
| if let Some(instance_id) = ctx.config().server()?.rivet.instance_id { | ||||||||||
| instance_id | ||||||||||
| } else { | ||||||||||
| Uuid::new_v4() | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| let cluster_id = CLUSTER_ID_ONCE | ||||||||||
| let instance_id = INSTANCE_ID_ONCE | ||||||||||
| .get_or_try_init(|| async { | ||||||||||
| sql_fetch_one!( | ||||||||||
| [ctx, (Uuid,)] | ||||||||||
| " | ||||||||||
| WITH new_row AS ( | ||||||||||
| INSERT INTO db_dynamic_config.config (id, cluster_id) | ||||||||||
| INSERT INTO db_dynamic_config.config (id, rivet_instance_id) | ||||||||||
| VALUES (1, $1) | ||||||||||
| ON CONFLICT (id) DO NOTHING | ||||||||||
| RETURNING cluster_id | ||||||||||
| RETURNING rivet_instance_id | ||||||||||
| ) | ||||||||||
| SELECT cluster_id | ||||||||||
| SELECT rivet_instance_id | ||||||||||
| FROM new_row | ||||||||||
| UNION ALL | ||||||||||
| SELECT cluster_id | ||||||||||
| SELECT rivet_instance_id | ||||||||||
| FROM db_dynamic_config.config | ||||||||||
| WHERE NOT EXISTS (SELECT 1 FROM new_row) | ||||||||||
| ", | ||||||||||
| default_cluster_id | ||||||||||
| default_instance_id | ||||||||||
| ) | ||||||||||
| .await | ||||||||||
| .map(|x| x.0) | ||||||||||
| }) | ||||||||||
| .await?; | ||||||||||
|
|
||||||||||
| Ok(Output { | ||||||||||
| cluster_id: *cluster_id, | ||||||||||
| instance_id: *instance_id, | ||||||||||
| }) | ||||||||||
| } | ||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -41,22 +41,22 @@ pub async fn run_from_env( | |||||||||||||
| (), | ||||||||||||||
| ); | ||||||||||||||
|
|
||||||||||||||
| // Get the cluster ID | ||||||||||||||
| let cluster_id = chirp_workflow::compat::op(&ctx, dynamic_config::ops::get_config::Input {}) | ||||||||||||||
| // Get the rivet instance ID | ||||||||||||||
| let instance_id = chirp_workflow::compat::op(&ctx, dynamic_config::ops::get_config::Input {}) | ||||||||||||||
| .await? | ||||||||||||||
| .cluster_id; | ||||||||||||||
| .instance_id; | ||||||||||||||
|
|
||||||||||||||
| // Build events | ||||||||||||||
| let mut events = Vec::new(); | ||||||||||||||
| let distinct_id = format!("cluster:{cluster_id}"); | ||||||||||||||
| let distinct_id = format!("instance:{instance_id}"); | ||||||||||||||
|
|
||||||||||||||
| // Send beacon | ||||||||||||||
| let mut event = async_posthog::Event::new("cluster_beacon", &distinct_id); | ||||||||||||||
| event.insert_prop("$groups", json!({ "cluster": cluster_id }))?; | ||||||||||||||
| let mut event = async_posthog::Event::new("instance_beacon", &distinct_id); | ||||||||||||||
| event.insert_prop("$groups", json!({ "instance": instance_id }))?; | ||||||||||||||
| event.insert_prop( | ||||||||||||||
| "$set", | ||||||||||||||
| json!({ | ||||||||||||||
| "cluster_id": cluster_id, | ||||||||||||||
| "instance_id": instance_id, | ||||||||||||||
| "os": os_report(), | ||||||||||||||
| "source_hash": rivet_env::source_hash(), | ||||||||||||||
| "config": get_config_data(&ctx)?, | ||||||||||||||
|
|
@@ -66,10 +66,10 @@ pub async fn run_from_env( | |||||||||||||
| )?; | ||||||||||||||
| events.push(event); | ||||||||||||||
|
|
||||||||||||||
| // Add cluster identification data | ||||||||||||||
| // Add instance identification data | ||||||||||||||
| let mut event = async_posthog::Event::new("$groupidentify", &distinct_id); | ||||||||||||||
| event.insert_prop("$group_type", "cluster")?; | ||||||||||||||
| event.insert_prop("$group_key", cluster_id)?; | ||||||||||||||
| event.insert_prop("$group_type", "instance")?; | ||||||||||||||
| event.insert_prop("$group_key", instance_id)?; | ||||||||||||||
| event.insert_prop( | ||||||||||||||
| "$group_set", | ||||||||||||||
| json!({ | ||||||||||||||
|
|
@@ -89,7 +89,7 @@ pub async fn run_from_env( | |||||||||||||
| Ok(()) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// Returns information about the operating system running the cluster. | ||||||||||||||
| /// Returns information about the operating system running the instance. | ||||||||||||||
| /// | ||||||||||||||
| /// This helps Rivet diagnose crash reports to easily pinpoint if issues are | ||||||||||||||
| /// coming from a specific operating system. | ||||||||||||||
|
|
@@ -190,9 +190,9 @@ struct ResourceAggregateRow { | |||||||||||||
|
|
||||||||||||||
| // /// Returns information about the pegboard configuration. | ||||||||||||||
| // /// | ||||||||||||||
| // /// This is helpful for diagnosing issues with the self-hosted clusters under | ||||||||||||||
| // /// load. e.g. if a cluster is running on constraint resources (see os_report), | ||||||||||||||
| // /// does the cluster configuration affect it? | ||||||||||||||
| // /// 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? | ||||||||||||||
|
Comment on lines
+193
to
+195
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. syntax: typo: 'constraint resources' should be 'constrained resources'
Suggested change
|
||||||||||||||
| // async fn get_pegboard_data(ctx: &OperationContext<()>) -> GlobalResult<serde_json::Value> { | ||||||||||||||
| // use pegboard::protocol::ClientFlavor; | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Documentation comment appears to be incorrect - refers to 'default cluster ID' but this is for instance_id