Skip to content

Commit 9fb7a6e

Browse files
authored
[persist] Postgres specific consensus queries (#32389)
This PR adds some new queries to `PostgresConsensus` that are tuned against vanilla postgres to obtain row level locks as opposed to the table level locks that the current queries get. These new queries are only used if `persist_use_postgres_tuned_queries` is set to true, __and__ we detect we're running against vanilla Postgres. Locks acquired on `main` ``` pid | mode | granted -------+------------------+--------- 21004 | AccessShareLock | t 21004 | RowExclusiveLock | t ``` Locks acquired with the new feature enabled ``` pid | mode | granted -------+------------------+--------- 21004 | RowShareLock | t 21004 | RowExclusiveLock | t ``` It's not entirely clear that this improves performance, but some simulation testing seems to indicate that it does. ### Testing Also included in this PR are new CI workflows that run platform-checks and maelstrom against Postgres consensus, using the new Postgres queries. ### Motivation Improve performance of self-hosted which runs against vanilla postgres ### Tips for reviewer <!-- Leave some tips for your reviewer, like: * The diff is much smaller if viewed with whitespace hidden. * [Some function/module/file] deserves extra attention. * [Some function/module/file] is pure code movement and only needs a skim. Delete this section if no tips. --> ### Checklist - [ ] This PR has adequate test coverage / QA involvement has been duly considered. ([trigger-ci for additional test/nightly runs](https://trigger-ci.dev.materialize.com/)) - [ ] This PR has an associated up-to-date [design doc](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/README.md), is a design doc ([template](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/00000000_template.md)), or is sufficiently small to not require a design. <!-- Reference the design in the description. --> - [ ] If this PR evolves [an existing `$T ⇔ Proto$T` mapping](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/command-and-response-binary-encoding.md) (possibly in a backwards-incompatible way), then it is tagged with a `T-proto` label. - [ ] If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label ([example](MaterializeInc/cloud#5021)). <!-- Ask in #team-cloud on Slack if you need help preparing the cloud PR. --> - [ ] If this PR includes major [user-facing behavior changes](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/guide-changes.md#what-changes-require-a-release-note), I have pinged the relevant PM to schedule a changelog post.
1 parent 7c1a99d commit 9fb7a6e

File tree

13 files changed

+239
-35
lines changed

13 files changed

+239
-35
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

ci/nightly/pipeline.template.yml

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,24 @@ steps:
787787
"--seed=$BUILDKITE_JOB_ID",
788788
]
789789

790+
- id: checks-parallel-drop-create-default-replica-postgres
791+
label: "Checks parallel + DROP/CREATE replica (:postgres: Consensus)"
792+
depends_on: build-aarch64
793+
timeout_in_minutes: 180
794+
parallelism: 2
795+
agents:
796+
queue: hetzner-aarch64-16cpu-32gb
797+
plugins:
798+
- ./ci/plugins/mzcompose:
799+
composition: platform-checks
800+
args:
801+
[
802+
--scenario=DropCreateDefaultReplica,
803+
--execution-mode=parallel,
804+
--features=postgres_consensus,
805+
--system-param=persist_use_postgres_tuned_queries=true,
806+
"--seed=$BUILDKITE_JOB_ID",
807+
]
790808

791809
- id: checks-parallel-restart-clusterd-compute
792810
label: "Checks parallel + restart compute clusterd"
@@ -824,6 +842,24 @@ steps:
824842
composition: platform-checks
825843
args: [--scenario=RestartEnvironmentdClusterdStorage, --execution-mode=parallel, "--seed=$BUILDKITE_JOB_ID"]
826844

845+
- id: checks-parallel-restart-environmentd-clusterd-storage-postgres-consensus
846+
label: "Checks parallel + restart of environmentd & storage clusterd (:postgres: Consensus)"
847+
depends_on: build-aarch64
848+
timeout_in_minutes: 180
849+
parallelism: 2
850+
agents:
851+
queue: hetzner-aarch64-16cpu-32gb
852+
plugins:
853+
- ./ci/plugins/mzcompose:
854+
composition: platform-checks
855+
args: [
856+
--scenario=RestartEnvironmentdClusterdStorage,
857+
--execution-mode=parallel,
858+
--features=postgres_consensus,
859+
--system-param=persist_use_postgres_tuned_queries=true,
860+
"--seed=$BUILDKITE_JOB_ID",
861+
]
862+
827863
- id: checks-parallel-kill-clusterd-storage
828864
label: "Checks parallel + kill storage clusterd"
829865
depends_on: build-aarch64
@@ -1107,7 +1143,7 @@ steps:
11071143
args: [--node-count=1, --consensus=mem, --blob=mem, --time-limit=600, --concurrency=4, --rate=500, --max-txn-length=16, --unreliability=0.1]
11081144

11091145
- id: persist-maelstrom-multi-node
1110-
label: Long multi-node Maelstrom coverage of persist with postgres consensus
1146+
label: Long multi-node Maelstrom coverage of persist with CockroachDB consensus
11111147
depends_on: build-aarch64
11121148
timeout_in_minutes: 40
11131149
agents:
@@ -1118,8 +1154,20 @@ steps:
11181154
composition: persist
11191155
args: [--node-count=4, --consensus=cockroach, --blob=maelstrom, --time-limit=300, --concurrency=4, --rate=500, --max-txn-length=16, --unreliability=0.1]
11201156

1157+
- id: persist-maelstrom-multi-node-postgres
1158+
label: "Long multi-node Maelstrom coverage of persist with :postgres: Consensus"
1159+
depends_on: build-aarch64
1160+
timeout_in_minutes: 40
1161+
agents:
1162+
queue: hetzner-aarch64-4cpu-8gb
1163+
artifact_paths: [test/persist/maelstrom/**/*.log]
1164+
plugins:
1165+
- ./ci/plugins/mzcompose:
1166+
composition: persist
1167+
args: [--node-count=4, --consensus=postgres, --blob=maelstrom, --time-limit=300, --concurrency=4, --rate=500, --max-txn-length=16, --unreliability=0.1]
1168+
11211169
- id: txn-wal-maelstrom
1122-
label: Maelstrom coverage of txn-wal
1170+
label: Maelstrom coverage of txn-wal with CockroachDB Consensus
11231171
depends_on: build-aarch64
11241172
timeout_in_minutes: 40
11251173
agents:
@@ -1130,6 +1178,18 @@ steps:
11301178
composition: persist
11311179
args: [--node-count=4, --consensus=cockroach, --blob=maelstrom, --time-limit=300, --rate=500, --txn-wal]
11321180

1181+
- id: txn-wal-maelstrom-postgres
1182+
label: "Maelstrom coverage of txn-wal with :postgres: Consensus"
1183+
depends_on: build-aarch64
1184+
timeout_in_minutes: 40
1185+
agents:
1186+
queue: hetzner-aarch64-8cpu-16gb
1187+
artifact_paths: [test/persist/maelstrom/**/*.log]
1188+
plugins:
1189+
- ./ci/plugins/mzcompose:
1190+
composition: persist
1191+
args: [--node-count=4, --consensus=postgres, --blob=maelstrom, --time-limit=300, --rate=500, --txn-wal]
1192+
11331193
- id: persistence-failpoints
11341194
label: Persistence failpoints
11351195
depends_on: build-aarch64

misc/python/materialize/checks/features.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
class Features:
1616
AZURITE = "azurite"
1717
SQL_SERVER = "sql_server"
18+
POSTGRES_CONSENSUS = "postgres_consensus"
1819

1920
def __init__(self, features):
2021
self.features = features
@@ -24,3 +25,6 @@ def azurite_enabled(self) -> bool:
2425

2526
def sql_server_enabled(self) -> bool:
2627
return self.features and self.SQL_SERVER in self.features
28+
29+
def postgres_consensus_enabled(self) -> bool:
30+
return self.features and self.POSTGRES_CONSENSUS in self.features

src/persist-cli/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ rust_binary(
3636
}),
3737
version = "0.0.0",
3838
deps = [
39+
"//src/dyncfg:mz_dyncfg",
3940
"//src/http-util:mz_http_util",
4041
"//src/orchestrator-tracing:mz_orchestrator_tracing",
4142
"//src/ore:mz_ore",

src/persist-cli/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ clap = { version = "4.5.23", features = ["derive", "env"] }
2626
differential-dataflow = "0.14.2"
2727
futures = "0.3.31"
2828
humantime = "2.2.0"
29+
mz-dyncfg = { path = "../dyncfg" }
2930
mz-http-util = { path = "../http-util" }
3031
mz-orchestrator-tracing = { path = "../orchestrator-tracing" }
3132
mz-ore = { path = "../ore", features = ["bytes", "network", "panic", "tracing", "test"] }

src/persist-cli/src/maelstrom/txn_list_append_multi.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH};
1919

2020
use async_trait::async_trait;
2121
use differential_dataflow::consolidation::consolidate_updates;
22+
use mz_dyncfg::ConfigUpdates;
2223
use mz_ore::metrics::MetricsRegistry;
2324
use mz_ore::now::{NOW_ZERO, SYSTEM_TIME};
2425
use mz_persist::cfg::{BlobConfig, ConsensusConfig};
@@ -372,6 +373,14 @@ impl Service for TransactorService {
372373

373374
let mut config =
374375
PersistConfig::new_default_configs(&mz_persist_client::BUILD_INFO, SYSTEM_TIME.clone());
376+
{
377+
// We only use the Postgres tuned queries when connected to vanilla
378+
// Postgres, so we always want to enable them for testing.
379+
let mut updates = ConfigUpdates::default();
380+
updates.add(&mz_persist::postgres::USE_POSTGRES_TUNED_QUERIES, true);
381+
config.apply_from(&updates);
382+
}
383+
375384
let metrics_registry = MetricsRegistry::new();
376385
let metrics = Arc::new(PersistMetrics::new(&config, &metrics_registry));
377386

@@ -417,6 +426,7 @@ impl Service for TransactorService {
417426
consensus_uri,
418427
Box::new(config.clone()),
419428
metrics.postgres_consensus.clone(),
429+
Arc::clone(&config.configs),
420430
)
421431
.expect("consensus_uri should be valid");
422432
loop {

src/persist-cli/src/maelstrom/txn_list_append_single.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH};
1717
use async_trait::async_trait;
1818
use differential_dataflow::consolidation::consolidate_updates;
1919
use differential_dataflow::lattice::Lattice;
20+
use mz_dyncfg::ConfigUpdates;
2021
use mz_ore::metrics::MetricsRegistry;
2122
use mz_ore::now::SYSTEM_TIME;
2223
use mz_persist::cfg::{BlobConfig, ConsensusConfig};
@@ -596,6 +597,14 @@ impl Service for TransactorService {
596597

597598
let mut config =
598599
PersistConfig::new_default_configs(&mz_persist_client::BUILD_INFO, SYSTEM_TIME.clone());
600+
{
601+
// We only use the Postgres tuned queries when connected to vanilla
602+
// Postgres, so we always want to enable them for testing.
603+
let mut updates = ConfigUpdates::default();
604+
updates.add(&mz_persist::postgres::USE_POSTGRES_TUNED_QUERIES, true);
605+
config.apply_from(&updates);
606+
}
607+
599608
let metrics = Arc::new(Metrics::new(&config, &MetricsRegistry::new()));
600609

601610
// Construct requested Blob.
@@ -640,6 +649,7 @@ impl Service for TransactorService {
640649
consensus_uri,
641650
Box::new(config.clone()),
642651
metrics.postgres_consensus.clone(),
652+
Arc::clone(&config.configs),
643653
)
644654
.expect("consensus_uri should be valid");
645655
loop {

src/persist-client/src/cache.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ impl PersistClientCache {
173173
x.key(),
174174
Box::new(self.cfg.clone()),
175175
self.metrics.postgres_consensus.clone(),
176+
Arc::clone(&self.cfg().configs),
176177
)?;
177178
let consensus =
178179
retry_external(&self.metrics.retries.external.consensus_open, || {

src/persist-client/src/cli/args.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ pub(super) async fn make_consensus(
130130
consensus_uri,
131131
Box::new(cfg.clone()),
132132
metrics.postgres_consensus.clone(),
133+
Arc::clone(&cfg.configs),
133134
)?;
134135
let consensus = consensus.clone().open().await?;
135136
let consensus = if commit {

src/persist/src/cfg.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ pub fn all_dyn_configs(configs: ConfigSet) -> ConfigSet {
3636
.add(&crate::indexed::columnar::arrow::ENABLE_ARROW_LGALLOC_NONCC_SIZES)
3737
.add(&crate::s3::ENABLE_S3_LGALLOC_CC_SIZES)
3838
.add(&crate::s3::ENABLE_S3_LGALLOC_NONCC_SIZES)
39+
.add(&crate::postgres::USE_POSTGRES_TUNED_QUERIES)
3940
}
4041

4142
/// Config for an implementation of [Blob].
@@ -233,10 +234,11 @@ impl ConsensusConfig {
233234
url: &SensitiveUrl,
234235
knobs: Box<dyn PostgresClientKnobs>,
235236
metrics: PostgresClientMetrics,
237+
dyncfg: Arc<ConfigSet>,
236238
) -> Result<Self, ExternalError> {
237239
let config = match url.scheme() {
238240
"postgres" | "postgresql" => Ok(ConsensusConfig::Postgres(
239-
PostgresConsensusConfig::new(url, knobs, metrics)?,
241+
PostgresConsensusConfig::new(url, knobs, metrics, dyncfg)?,
240242
)),
241243
"mem" => {
242244
if !cfg!(debug_assertions) {

0 commit comments

Comments
 (0)