Skip to content

Comments

fix: force RUST_MIN_STACK to 8MB for replica#1940

Merged
mergify[bot] merged 5 commits intomasterfrom
ericswanson/dfx-start-rust-min-stack
Dec 1, 2021
Merged

fix: force RUST_MIN_STACK to 8MB for replica#1940
mergify[bot] merged 5 commits intomasterfrom
ericswanson/dfx-start-rust-min-stack

Conversation

@ghost
Copy link

@ghost ghost commented Nov 24, 2021

For discussion.

  • Is this hardcoded value ok?
  • Should we only set it if RUST_MIN_STACK is not already set in the environment? (of course dfx it itself a rust application, so would be using the same value from the same environment variable)
  • Should we add a new command-line argument to dfx start? (which would apply only when using the replica, not --emulator)

@ghost ghost requested review from adambratschikaye, chenyan-dfinity and ninegua and removed request for ninegua November 24, 2021 18:00
@adambratschikaye
Copy link
Contributor

I think we have to go with a hard coded value, at least for now, just because there's no easy way to get the value that's used in the mainnet. That value is stored in ic-os/guestos/rootfs/etc/systemd/system/ic-replica.service (in the main ic repo) and already has a comment that the nix environment needs to be updated when the value changes. So we could add an additional comment about dfx needing to be updated.

I'm not sure what we want to do if the user has set the RUST_MIN_STACK themselves. We're only changing the environment only for the replica process, so all other running rust code will have the stack size they've set and this seems ok to me.

I don't have anything against an additional argument, but I'm not sure if anyone would actually want to use it.

@chenyan-dfinity
Copy link
Contributor

Maybe worth adding the following code to our e2e test. When the replica increase the stack size in the future, this test case will fail.

(module
  (export "canister_init" (func $init))
  (func $init
    (call $init)
  )
)

@ghost ghost added the automerge-squash label Dec 1, 2021
@mergify mergify bot merged commit 7c9241d into master Dec 1, 2021
@mergify mergify bot deleted the ericswanson/dfx-start-rust-min-stack branch December 1, 2021 23:06
@mergify mergify bot removed the automerge-squash label Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants