Skip to content

Commit

Permalink
Add support for using diesel async for migrations to avoid pq-sys dep
Browse files Browse the repository at this point in the history
  • Loading branch information
banool committed Mar 21, 2024
1 parent 48d7794 commit 3fe9b37
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 8 deletions.
5 changes: 5 additions & 0 deletions .github/workflows/lint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,8 @@ jobs:
rustup component add rustfmt --toolchain nightly
scripts/rust_lint.sh --check
working-directory: rust
- run: bash scripts/check_banned_deps.sh
working-directory: rust
- name: Ensure the --no-default-features build passes too
run: cargo build --no-default-features
working-directory: rust
17 changes: 15 additions & 2 deletions rust/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 6 additions & 2 deletions rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,18 @@ bcs = { git = "https://github.com/aptos-labs/bcs.git", rev = "d31fab9d81748e2594
bigdecimal = { version = "0.4.0", features = ["serde"] }
chrono = { version = "0.4.19", features = ["clock", "serde"] }
clap = { version = "4.3.5", features = ["derive", "unstable-styles"] }
# Do NOT enable the postgres feature here, it is conditionally enabled in a feature
# block in the Cargo.toml file for the processor crate.
# https://github.com/aptos-labs/aptos-indexer-processors/pull/325
diesel = { version = "2.1", features = [
"chrono",
"postgres_backend",
"numeric",
"postgres",
"serde_json",
] }
diesel-async = { version = "0.4", features = ["postgres", "bb8", "tokio"] }
# Use the crate version once this feature gets released on crates.io:
# https://github.com/weiznich/diesel_async/commit/e165e8c96a6c540ebde2d6d7c52df5c5620a4bf1
diesel-async = { git = "https://github.com/weiznich/diesel_async.git", rev = "d02798c67065d763154d7272dd0c09b39757d0f2", features = ["async-connection-wrapper", "postgres", "bb8", "tokio"] }
diesel_migrations = { version = "2.1.0", features = ["postgres"] }
diesel_async_migrations = { git = "https://github.com/niroco/diesel_async_migrations", rev = "11f331b73c5cfcc894380074f748d8fda710ac12" }
enum_dispatch = "0.3.12"
Expand Down
7 changes: 7 additions & 0 deletions rust/processor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,10 @@ url = { workspace = true }
native-tls = { workspace = true }
postgres-native-tls = { workspace = true }
tokio-postgres = { workspace = true }

[features]
libpq = ["diesel/postgres"]
# When using the default features we enable the diesel/postgres feature. We configure
# it in a feature so the CLI can opt out, since it cannot tolerate the libpq dep.
# Recall that features should always be additive.
default = ["libpq"]
2 changes: 1 addition & 1 deletion rust/processor/src/utils/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ where
Ok(())
}

pub async fn run_pending_migrations<DB: Backend>(conn: &mut impl MigrationHarness<DB>) {
pub fn run_pending_migrations<DB: Backend>(conn: &mut impl MigrationHarness<DB>) {
conn.run_pending_migrations(MIGRATIONS)
.expect("[Parser] Migrations failed!");
}
Expand Down
36 changes: 33 additions & 3 deletions rust/processor/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ use crate::{
use ahash::AHashMap;
use anyhow::{Context, Result};
use aptos_moving_average::MovingAverage;
use diesel::Connection;
use tokio::task::JoinHandle;
use tracing::{debug, error, info};
use url::Url;
Expand Down Expand Up @@ -495,13 +494,44 @@ impl Worker {
})
}

// For the normal processor build we just use standard Diesel with the postgres
// feature enabled (which uses libpq under the hood, hence why we named the feature
// this way).
#[cfg(feature = "libpq")]
async fn run_migrations(&self) {
use crate::diesel::Connection;
use diesel::pg::PgConnection;

println!("Running migrations: {:?}", self.postgres_connection_string);
info!("Running migrations: {:?}", self.postgres_connection_string);
let mut conn =
PgConnection::establish(&self.postgres_connection_string).expect("migrations failed!");
run_pending_migrations(&mut conn).await;
run_pending_migrations(&mut conn);
}

// If the libpq feature isn't enabled, we use diesel async instead. This is used by
// the CLI for the local testnet, where we cannot tolerate the libpq dependency.
#[cfg(not(feature = "libpq"))]
async fn run_migrations(&self) {
use diesel_async::async_connection_wrapper::AsyncConnectionWrapper;

info!("Running migrations: {:?}", self.postgres_connection_string);
let conn = self
.db_pool
// We need to use this since AsyncConnectionWrapper doesn't know how to
// work with a pooled connection.
.dedicated_connection()
.await
.expect("[Parser] Failed to get connection");
// We use spawn_blocking since run_pending_migrations is a blocking function.
tokio::task::spawn_blocking(move || {
// This lets us use the connection like a normal diesel connection. See more:
// https://docs.rs/diesel-async/latest/diesel_async/async_connection_wrapper/type.AsyncConnectionWrapper.html
let mut conn: AsyncConnectionWrapper<diesel_async::AsyncPgConnection> =
AsyncConnectionWrapper::from(conn);
run_pending_migrations(&mut conn);
})
.await
.expect("[Parser] Failed to run migrations");
}

/// Gets the start version for the processor. If not found, start from 0.
Expand Down
52 changes: 52 additions & 0 deletions rust/scripts/check_banned_deps.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#!/bin/sh

# This script checks if the crate depends on external deps that it shouldn't. We run
# this in CI to make sure we don't accidentally reintroduce deps that would make the
# crate unusable for the CLI.
#
# While it would be more reliable to actually build the crate and check what libraries
# it links to, e.g. with otool, it is much cheaper to use cargo tree. As far as I can
# tell the entire Rust ecosystem makes use of these `x-sys` libraries to depend on
# external dynamically linked libraries.
#
# We can almost use cargo deny but it doesn't support checking specific build paths. We
# don't care if openssl-sys for example is used at build time (which it is, indirectly
# by shadow-rs), only at run time. See more here:
# https://github.com/EmbarkStudios/cargo-deny/issues/563
#
# It assumes cargo and friends are available.
#
# Run this from the rust/ directory.

cd "$(dirname "$0")"
cd ..

declare -a deps=("pq-sys" "openssl-sys")

for dep in "${deps[@]}"; do
echo "Checking for banned dependency $dep..."

# Check for deps. As you can see, we only check for MacOS right now.
# We specify --no-default-features because we only care about these banned deps
# for the local testnet use case, in which case it opts out of the default
# features.
out=`cargo tree --no-default-features -e features,no-build,no-dev --target aarch64-apple-darwin -p processor -i "$dep"`

# If the exit status was non-zero, great, the dep couldn't be found.
if [ $? -ne 0 ]; then
continue
fi

# If the exit status was zero we have to check the output to see if the dep is in
# use. If it is in the output, it is in use.
if [[ $out != *"$dep"* ]]; then
continue
fi

echo "Banned dependency $dep found!"
exit 1
done

echo
echo "None of the banned dependencies are in use, great!"
exit 0

0 comments on commit 3fe9b37

Please sign in to comment.