-
-
Notifications
You must be signed in to change notification settings - Fork 3
feat(v2): Postgres storage adapter #529
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| CREATE DATABASE taskbroker; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| -- PostgreSQL equivalent of the inflight_taskactivations table | ||
| CREATE TABLE IF NOT EXISTS inflight_taskactivations ( | ||
| id TEXT NOT NULL PRIMARY KEY, | ||
| activation BYTEA NOT NULL, | ||
| partition INTEGER NOT NULL, | ||
| kafka_offset BIGINT NOT NULL, | ||
| added_at TIMESTAMPTZ NOT NULL, | ||
| received_at TIMESTAMPTZ NOT NULL, | ||
| processing_attempts INTEGER NOT NULL, | ||
| expires_at TIMESTAMPTZ, | ||
| delay_until TIMESTAMPTZ, | ||
| processing_deadline_duration INTEGER NOT NULL, | ||
| processing_deadline TIMESTAMPTZ, | ||
| status TEXT NOT NULL, | ||
| at_most_once BOOLEAN NOT NULL DEFAULT FALSE, | ||
| application TEXT NOT NULL DEFAULT '', | ||
| namespace TEXT NOT NULL, | ||
| taskname TEXT NOT NULL, | ||
| on_attempts_exceeded INTEGER NOT NULL DEFAULT 1 | ||
| ); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -121,6 +121,14 @@ pub struct Config { | |
| /// The number of ms for timeouts when publishing messages to kafka. | ||
| pub kafka_send_timeout_ms: u64, | ||
|
|
||
| pub database_adapter: &'static str, | ||
|
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. Why is this not an enum ? |
||
|
|
||
| /// The url of the postgres database to use for the inflight activation store. | ||
| pub pg_url: String, | ||
|
|
||
| /// The name of the postgres database to use for the inflight activation store. | ||
| pub pg_database_name: String, | ||
|
|
||
| /// The path to the sqlite database | ||
| pub db_path: String, | ||
|
|
||
|
|
@@ -256,6 +264,9 @@ impl Default for Config { | |
| kafka_auto_offset_reset: "latest".to_owned(), | ||
| kafka_send_timeout_ms: 500, | ||
| db_path: "./taskbroker-inflight.sqlite".to_owned(), | ||
| database_adapter: "sqlite", | ||
| pg_url: "postgres://postgres:password@sentry-postgres-1:5432/".to_owned(), | ||
| pg_database_name: "taskbroker".to_owned(), | ||
| db_write_failure_backoff_ms: 4000, | ||
| db_insert_batch_max_len: 256, | ||
| db_insert_batch_max_size: 16_000_000, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| use crate::grpc::server::TaskbrokerServer; | ||
| use crate::store::inflight_activation::InflightActivationStore; | ||
| use prost::Message; | ||
| use rstest::rstest; | ||
| use sentry_protos::taskbroker::v1::consumer_service_server::ConsumerService; | ||
| use sentry_protos::taskbroker::v1::{ | ||
| FetchNextTask, GetTaskRequest, SetTaskStatusRequest, TaskActivation, | ||
|
|
@@ -10,8 +10,11 @@ use tonic::{Code, Request}; | |
| use crate::test_utils::{create_test_store, make_activations}; | ||
|
|
||
| #[tokio::test] | ||
| async fn test_get_task() { | ||
| let store = create_test_store().await; | ||
| #[rstest] | ||
| #[case::sqlite("sqlite")] | ||
| #[case::postgres("postgres")] | ||
| async fn test_get_task(#[case] adapter: &str) { | ||
| let store = create_test_store(adapter).await; | ||
| let service = TaskbrokerServer { store }; | ||
| let request = GetTaskRequest { | ||
| namespace: None, | ||
|
|
@@ -25,9 +28,12 @@ async fn test_get_task() { | |
| } | ||
|
|
||
| #[tokio::test] | ||
| #[rstest] | ||
| #[case::sqlite("sqlite")] | ||
| #[case::postgres("postgres")] | ||
| #[allow(deprecated)] | ||
| async fn test_set_task_status() { | ||
| let store = create_test_store().await; | ||
| async fn test_set_task_status(#[case] adapter: &str) { | ||
| let store = create_test_store(adapter).await; | ||
| let service = TaskbrokerServer { store }; | ||
| let request = SetTaskStatusRequest { | ||
| id: "test_task".to_string(), | ||
|
|
@@ -41,9 +47,12 @@ async fn test_set_task_status() { | |
| } | ||
|
|
||
| #[tokio::test] | ||
| #[rstest] | ||
| #[case::sqlite("sqlite")] | ||
| #[case::postgres("postgres")] | ||
| #[allow(deprecated)] | ||
| async fn test_set_task_status_invalid() { | ||
| let store = create_test_store().await; | ||
| async fn test_set_task_status_invalid(#[case] adapter: &str) { | ||
| let store = create_test_store(adapter).await; | ||
| let service = TaskbrokerServer { store }; | ||
| let request = SetTaskStatusRequest { | ||
| id: "test_task".to_string(), | ||
|
|
@@ -61,9 +70,12 @@ async fn test_set_task_status_invalid() { | |
| } | ||
|
|
||
| #[tokio::test] | ||
| #[rstest] | ||
| #[case::sqlite("sqlite")] | ||
| #[case::postgres("postgres")] | ||
| #[allow(deprecated)] | ||
| async fn test_get_task_success() { | ||
| let store = create_test_store().await; | ||
| async fn test_get_task_success(#[case] adapter: &str) { | ||
| let store = create_test_store(adapter).await; | ||
| let activations = make_activations(1); | ||
| store.store(activations).await.unwrap(); | ||
|
|
||
|
|
@@ -81,9 +93,12 @@ async fn test_get_task_success() { | |
| } | ||
|
|
||
| #[tokio::test] | ||
| #[rstest] | ||
| #[case::sqlite("sqlite")] | ||
| #[case::postgres("postgres")] | ||
| #[allow(deprecated)] | ||
| async fn test_get_task_with_application_success() { | ||
| let store = create_test_store().await; | ||
| async fn test_get_task_with_application_success(#[case] adapter: &str) { | ||
| let store = create_test_store(adapter).await; | ||
| let mut activations = make_activations(2); | ||
|
|
||
| let mut payload = TaskActivation::decode(&activations[1].activation as &[u8]).unwrap(); | ||
|
|
@@ -108,9 +123,12 @@ async fn test_get_task_with_application_success() { | |
| } | ||
|
|
||
| #[tokio::test] | ||
| #[rstest] | ||
| #[case::sqlite("sqlite")] | ||
| #[case::postgres("postgres")] | ||
| #[allow(deprecated)] | ||
| async fn test_get_task_with_namespace_requires_application() { | ||
| let store = create_test_store().await; | ||
| async fn test_get_task_with_namespace_requires_application(#[case] adapter: &str) { | ||
| let store = create_test_store(adapter).await; | ||
| let activations = make_activations(2); | ||
| let namespace = activations[0].namespace.clone(); | ||
|
|
||
|
|
@@ -129,9 +147,12 @@ async fn test_get_task_with_namespace_requires_application() { | |
| } | ||
|
|
||
| #[tokio::test] | ||
| #[rstest] | ||
| #[case::sqlite("sqlite")] | ||
| #[case::postgres("postgres")] | ||
| #[allow(deprecated)] | ||
| async fn test_set_task_status_success() { | ||
| let store = create_test_store().await; | ||
| async fn test_set_task_status_success(#[case] adapter: &str) { | ||
| let store = create_test_store(adapter).await; | ||
| let activations = make_activations(2); | ||
| store.store(activations).await.unwrap(); | ||
|
|
||
|
|
@@ -157,6 +178,7 @@ async fn test_set_task_status_success() { | |
| }), | ||
| }; | ||
| let response = service.set_task_status(Request::new(request)).await; | ||
| println!("response: {:?}", response); | ||
|
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. please remove |
||
| assert!(response.is_ok()); | ||
| let resp = response.unwrap(); | ||
| assert!(resp.get_ref().task.is_some()); | ||
|
|
@@ -165,9 +187,12 @@ async fn test_set_task_status_success() { | |
| } | ||
|
|
||
| #[tokio::test] | ||
| #[rstest] | ||
| #[case::sqlite("sqlite")] | ||
| #[case::postgres("postgres")] | ||
| #[allow(deprecated)] | ||
| async fn test_set_task_status_with_application() { | ||
| let store = create_test_store().await; | ||
| async fn test_set_task_status_with_application(#[case] adapter: &str) { | ||
| let store = create_test_store(adapter).await; | ||
| let mut activations = make_activations(2); | ||
|
|
||
| let mut payload = TaskActivation::decode(&activations[1].activation as &[u8]).unwrap(); | ||
|
|
@@ -199,9 +224,12 @@ async fn test_set_task_status_with_application() { | |
| } | ||
|
|
||
| #[tokio::test] | ||
| #[rstest] | ||
| #[case::sqlite("sqlite")] | ||
| #[case::postgres("postgres")] | ||
| #[allow(deprecated)] | ||
| async fn test_set_task_status_with_application_no_match() { | ||
| let store = create_test_store().await; | ||
| async fn test_set_task_status_with_application_no_match(#[case] adapter: &str) { | ||
| let store = create_test_store(adapter).await; | ||
| let mut activations = make_activations(2); | ||
|
|
||
| let mut payload = TaskActivation::decode(&activations[1].activation as &[u8]).unwrap(); | ||
|
|
@@ -228,9 +256,12 @@ async fn test_set_task_status_with_application_no_match() { | |
| } | ||
|
|
||
| #[tokio::test] | ||
| #[rstest] | ||
| #[case::sqlite("sqlite")] | ||
| #[case::postgres("postgres")] | ||
| #[allow(deprecated)] | ||
| async fn test_set_task_status_with_namespace_requires_application() { | ||
| let store = create_test_store().await; | ||
| async fn test_set_task_status_with_namespace_requires_application(#[case] adapter: &str) { | ||
| let store = create_test_store(adapter).await; | ||
| let activations = make_activations(2); | ||
| let namespace = activations[0].namespace.clone(); | ||
|
|
||
|
|
||
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.
This may not need to have a default now that it has been merged and rolled out.