Skip to content
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

Add support for using diesel async for migrations to avoid pq-sys dep #325

Merged
merged 1 commit into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading