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.

Sign up for GitHub

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

Stacker stack size experiment #30541

Closed
wants to merge 1 commit into from
Closed

Stacker stack size experiment #30541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
46 changes: 31 additions & 15 deletions Cargo.lock
View file
Open in desktop

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

9 changes: 0 additions & 9 deletions clippy.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,25 @@ await-holding-invalid-types = [
disallowed-methods = [
{ path = "std::panic::catch_unwind", reason = "use `mz_ore::panic::catch_unwind` instead" },
{ path = "futures::FutureExt::catch_unwind", reason = "use `mz_ore::future::FutureExt::catch_unwind` instead" },

{ path = "futures_executor::block_on", reason = "use `tokio::runtime::Handle::block_on` instead" },
{ path = "futures::executor::block_on", reason = "use `tokio::runtime::Handle::block_on` instead" },

{ path = "tokio::spawn", reason = "use the spawn wrappers in `mz_ore::task` instead" },
{ path = "tokio::task::spawn", reason = "use the spawn wrappers in `mz_ore::task` instead" },
{ path = "tokio::task::spawn_blocking", reason = "use the spawn wrappers in `mz_ore::task` instead" },
{ path = "tokio::runtime::Handle::spawn", reason = "use the spawn wrappers in `mz_ore::task` instead" },
{ path = "tokio::runtime::Handle::spawn_blocking", reason = "use the spawn wrappers in `mz_ore::task` instead" },
{ path = "tokio::runtime::Runtime::spawn", reason = "use the spawn wrappers in `mz_ore::task` instead" },
{ path = "tokio::runtime::Runtime::spawn_blocking", reason = "use the spawn wrappers in `mz_ore::task` instead" },

# Note these wrappers aren't implemented yet, as we haven't needed them.
{ path = "tokio::task::spawn_local", reason = "use the spawn wrappers in `mz_ore::task` instead" },
{ path = "tokio::task::LocalSet::spawn_local", reason = "use the spawn wrappers in `mz_ore::task` instead" },

# Note that `spawn_blocking` and the local varieties are not yet implemented, as they haven't been needed yet.
{ path = "tokio::task::JoinSet::spawn", reason = "use the spawn wrappers in `mz_ore::task` instead" },
{ path = "tokio::task::JoinSet::spawn_on", reason = "use the spawn wrappers in `mz_ore::task` instead" },
{ path = "tokio::task::JoinSet::spawn_blocking", reason = "use the spawn wrappers in `mz_ore::task` instead" },
{ path = "tokio::task::JoinSet::spawn_blocking_on", reason = "use the spawn wrappers in `mz_ore::task` instead" },
{ path = "tokio::task::JoinSet::spawn_local", reason = "use the spawn wrappers in `mz_ore::task` instead" },
{ path = "tokio::task::JoinSet::spawn_local", reason = "use the spawn wrappers in `mz_ore::task` instead" },

# These are banned because we want to ensure people don't forget to use .name(...), so we require the use of `mz_ore`
{ path = "tokio::task::Builder::spawn", reason = "use the spawn wrappers in `mz_ore::task` instead" },
{ path = "tokio::task::Builder::spawn_on", reason = "use the spawn wrappers in `mz_ore::task` instead" },
Expand All @@ -41,15 +36,12 @@ disallowed-methods = [
{ path = "tokio::task::join_set::Builder::spawn_on", reason = "use the spawn wrappers in `mz_ore::task` instead" },
{ path = "tokio::task::join_set::Builder::spawn_local", reason = "use the spawn wrappers in `mz_ore::task` instead" },
{ path = "tokio::task::join_set::Builder::spawn_local", reason = "use the spawn wrappers in `mz_ore::task` instead" },

{ path = "rdkafka::config::ClientConfig::new", reason = "use the `client::create_new_client_config` wrapper in `kafka_util` instead" },

{ path = "aws_config::defaults", reason = "use the `mz_aws_config::defaults` function instead" },
{ path = "aws_config::load_defaults", reason = "use the `mz_aws_config::defaults` function instead" },
{ path = "aws_config::from_env", reason = "use the `mz_aws_config::defaults` function instead" },
{ path = "aws_config::load_from_env", reason = "use the `mz_aws_config::defaults` function instead" },
{ path = "aws_sdk_s3::Client::new", reason = "use the `mz_aws_util::s3::new_client` function instead" },

# Prevent access to Differential APIs that want to use the default trace or use a default name, or where we offer
# our own wrapper
{ path = "differential_dataflow::Collection::consolidate", reason = "use the `mz_timely_util::operator::CollectionExt::consolidate_named` function instead" },
Expand Down Expand Up @@ -77,7 +69,6 @@ disallowed-methods = [
{ path = "differential_dataflow::operators::reduce::join::Join::antijoin", reason = "use the `differential_dataflow::operators::join::Join::join_core` function instead" },
{ path = "differential_dataflow::operators::reduce::join::Join::join_map", reason = "use the `differential_dataflow::operators::join::Join::join_core` function instead" },
{ path = "differential_dataflow::operators::reduce::join::Join::semijoin", reason = "use the `differential_dataflow::operators::join::Join::join_core` function instead" },

# Prevent access to Timely APIs with untested semantics.
{ path = "timely::worker::Worker::drop_dataflow", reason = "Might break logging dataflows, check with #team-compute." },
]
Expand Down
75 changes: 38 additions & 37 deletions src/ore/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ anyhow = { version = "1.0.66", optional = true }
async-trait = { version = "0.1.68", optional = true }
bytes = { version = "1.3.0", optional = true }
chrono = { version = "0.4.35", default-features = false, features = [
"std",
"std",
], optional = true }
backtrace = "0.3.74"
clap = { version = "3.2.24", features = ["env"], optional = true }
compact_bytes = { version = "0.1.2", optional = true }
ctor = { version = "0.1.26", optional = true }
Expand All @@ -43,19 +44,19 @@ paste = "1.0.11"
pin-project = "1.0.12"
prometheus = { version = "0.13.3", default-features = false, optional = true }
proptest = { version = "1.0.0", default-features = false, features = [
"std",
"std",
], optional = true }
rand = { version = "0.8.5", optional = true }
smallvec = { version = "1.10.0", optional = true }
stacker = { version = "0.1.15", optional = true }
sentry = { version = "0.29.1", optional = true, features = ["debug-images"] }
serde = { version = "1.0.152", features = ["derive"] }
tokio = { version = "1.38.0", features = [
"io-util",
"net",
"rt-multi-thread",
"sync",
"time",
"io-util",
"net",
"rt-multi-thread",
"sync",
"time",
], optional = true }
tokio-openssl = { version = "0.6.3", optional = true }
thiserror = "1.0.37"
Expand All @@ -67,10 +68,10 @@ tracing-capture = { version = "0.1.0", optional = true }
# Note that this feature is distinct from `tracing`'s `log` feature, which has `tracing` macros emit `log` records if
# there is no global `tracing` subscriber.
tracing-subscriber = { version = "0.3.16", default-features = false, features = [
"env-filter",
"fmt",
"json",
"tracing-log",
"env-filter",
"fmt",
"json",
"tracing-log",
], optional = true }
uuid = { version = "1.7.0", optional = true }
url = { version = "2.3.1", features = ["serde"] }
Expand All @@ -88,7 +89,7 @@ hyper-util = { version = "0.1.6", optional = true }
opentelemetry = { version = "0.24.0", features = ["trace"], optional = true }
opentelemetry-otlp = { version = "0.17.0", optional = true }
opentelemetry_sdk = { version = "0.24.1", features = [
"rt-tokio",
"rt-tokio",
], optional = true }
console-subscriber = { version = "0.4.0", optional = true }
sentry-tracing = { version = "0.29.1", optional = true }
Expand All @@ -108,37 +109,37 @@ tracing-subscriber = "0.3.16"
[features]
default = ["tokio-console", "workspace-hack", "mz-ore-proc/workspace-hack"]
async = [
"async-trait",
"futures",
"metrics",
"openssl",
"tokio-openssl",
"tokio",
"tracing",
"async-trait",
"futures",
"metrics",
"openssl",
"tokio-openssl",
"tokio",
"tracing",
]
bytes_ = ["bytes", "compact_bytes", "smallvec", "smallvec/const_generics", "region", "tracing_"]
network = ["async", "bytes", "smallvec", "tonic", "tracing"]
process = ["libc"]
region = ["lgalloc"]
tracing_ = [
"anyhow",
"tracing",
"tracing-subscriber",
"tracing-subscriber/ansi",
"tracing-opentelemetry",
"tokio-native-tls",
"native-tls",
"http",
"hyper-tls",
"hyper-util",
"metrics",
"opentelemetry",
"opentelemetry-otlp",
"opentelemetry_sdk",
"tonic",
"sentry",
"sentry-tracing",
"yansi",
"anyhow",
"tracing",
"tracing-subscriber",
"tracing-subscriber/ansi",
"tracing-opentelemetry",
"tokio-native-tls",
"native-tls",
"http",
"hyper-tls",
"hyper-util",
"metrics",
"opentelemetry",
"opentelemetry-otlp",
"opentelemetry_sdk",
"tonic",
"sentry",
"sentry-tracing",
"yansi",
]
tokio-console = ["console-subscriber", "tokio", "tokio/tracing", "network"]
cli = ["clap"]
Expand Down
38 changes: 38 additions & 0 deletions src/ore/src/stack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,44 @@ pub fn maybe_grow<F, R>(f: F) -> R
where
F: FnOnce() -> R,
{
if stacker::remaining_stack()
.map(|r| r < STACK_RED_ZONE)
.unwrap_or(false)
{
#[inline(never)]
fn trace() {
let mut limit = None;
let mut last_sp = None;
let mut depth = 0;
backtrace::trace(|frame| {
if limit.is_none() {
limit = Some(frame.sp().wrapping_add(STACK_SIZE));
eprintln!("Stack limit: {:?}", limit.unwrap());
}

let frame_size =
unsafe { frame.sp().byte_offset_from(last_sp.unwrap_or(frame.sp())) };

// Resolve this instruction pointer to a symbol name
backtrace::resolve_frame(frame, |symbol| {
eprintln!(
"{depth}\t{:?}\t{frame_size}\t{:?}, at {}:{}",
frame.sp(),
symbol.name(),
symbol.filename().and_then(|s| s.to_str()).unwrap_or("???"),
symbol.lineno().unwrap_or(0),
);
});

last_sp = Some(frame.sp());

depth += 1;

frame.sp() <= limit.unwrap()
});
trace();
}
}
stacker::maybe_grow(STACK_RED_ZONE, STACK_SIZE, f)
}

Expand Down
19 changes: 12 additions & 7 deletions src/sql/src/func.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1649,14 +1649,19 @@ macro_rules! builtins {

let mut builtins = BTreeMap::new();
$(
let impls = vec![$(impl_def!($params, $op, $return_type, $oid)),+];
let func = Func::$ty(impls);
let expect_set_return = matches!(&func, Func::Table(_));
for imp in func.func_impls() {
assert_eq!(imp.return_is_set, expect_set_return, "wrong set return value for func with oid {}", imp.oid);
{
fn insert_fn(builtins: &mut BTreeMap<&'static str, Func>) {
let impls = vec![$(impl_def!($params, $op, $return_type, $oid)),+];
let func = Func::$ty(impls);
let expect_set_return = matches!(&func, Func::Table(_));
for imp in func.func_impls() {
assert_eq!(imp.return_is_set, expect_set_return, "wrong set return value for func with oid {}", imp.oid);
}
let old = builtins.insert($name, func);
mz_ore::assert_none!(old, "duplicate entry in builtins list");
}
insert_fn(&mut builtins);
}
let old = builtins.insert($name, func);
mz_ore::assert_none!(old, "duplicate entry in builtins list");
)+
builtins
}};
Expand Down
Loading