Various minor testnet demo script tweaks #366

mvines · 2018-06-15T22:37:48Z

Permit the testnet to run all on the same machine/workspace (leader, validator, client) for quick tests
Only sudo on Linux
Tiny error reporting improvements, as an excuse to do something in Rust.

mvines · 2018-06-15T22:45:56Z

Looks like I need to install fmt...

rwalker-com · 2018-06-15T22:57:32Z

multinode-demo/client.sh

@@ -9,7 +9,9 @@ LEADER=$1
 COUNT=${2:-1}

 set -x
-rsync -v -e ssh "$LEADER"/{leader.json,mint-demo.json} .
+if [[ "$LEADER" != "localhost" ]]; then
+  rsync -v -e ssh "$LEADER"/{leader.json,mint-demo.json} .


turns out that using rsync locally works fine too... I've been doing:

mkdir client
cd client
../multinode-demo/client.sh .. 1

works fine

Ah, nice. ./multinode-demo/client.sh . 1 works too (I wanted to run everything from the same directory), will update this

rwalker-com · 2018-06-15T22:58:11Z

multinode-demo/leader.sh

+if [[ "$(uname)" = "Linux" ]]; then
+  (
+    set -x
+    sudo sysctl -w net.core.rmem_max=26214400


insufficient for WSL, which is also uname Linux, but doesn't have this sysctl... again, I just left it

Really?! I'm not sure how I feel about that just yet, it's surprising though!

rwalker-com · 2018-06-15T22:59:22Z

multinode-demo/leader.sh

-sudo sysctl -w net.core.rmem_max=26214400
+if [[ "$(uname)" = "Linux" ]]; then
+  (
+    set -x


why would you limit set -x to this subshell?

Less stdout I guess, but you're right. No point in limiting

rwalker-com · 2018-06-15T23:00:00Z

src/bin/client-demo.rs

@@ -395,6 +395,8 @@ fn converge(
 }

 fn read_leader(path: String) -> ReplicatedData {
-    let file = File::open(path).expect("file");
-    serde_json::from_reader(file).expect("parse")
+    let file = File::open(path.clone())


looks like someone's been reading their rust book... can we get there with a borrow instead?

Yeah, let file = File::open(&path) works too. I reached for that first ironically, but then I saw https://github.com/solana-labs/solana/blob/master/src/bin/fullnode.rs#L114 and cargo culted in the .clone().

understanding borrow is my #1 rust Q right now

rwalker-com · 2018-06-15T23:02:59Z

src/bin/fullnode.rs

@@ -115,14 +115,19 @@ fn main() {
            if let Ok(data) = serde_json::from_reader(file) {
                repl_data = data;
            } else {
-                warn!("failed to parse leader {}, generating new identity", path);


I don't like thie behavior, would lobby for predictability. i.e. if the file isn't there or is unparse-able, don't try to mask and do the "right thing", it only confuses a user who's mis-typed instructions to find an identity.

this is especially true in a project that is so verbose about info logging, warnings get lost in the noise, users who are making mistakes on the command line don't realize it, hell: can't realize it...

yep, it bugged me a little too but decided that was out of scope for my first babystep Rust PR. I filed #367 so we can follow up on this sometime

rwalker-com · 2018-06-15T23:04:10Z

heh, I failed to format once, too. once.

rwalker-com · 2018-06-15T23:04:44Z

great branchname

codecov · 2018-06-15T23:31:40Z

Codecov Report

Merging #366 into master will increase coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #366      +/-   ##
==========================================
+ Coverage   90.75%   90.81%   +0.05%     
==========================================
  Files          36       36              
  Lines        3484     3484              
==========================================
+ Hits         3162     3164       +2     
+ Misses        322      320       -2

Impacted Files	Coverage Δ
src/crdt.rs	`84.49% <0%> (+0.28%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 327ee1d...7d84373. Read the comment docs.

rob-solana · 2018-06-15T23:37:11Z

multinode-demo/validator.sh

@@ -11,7 +11,9 @@ set -x

 rsync -v -e ssh "$LEADER"/{mint-demo.json,leader.json,genesis.log} . || exit $?

-sudo sysctl -w net.core.rmem_max=26214400
+if [[ "$(uname)" = "Linux" ]]; then


since this doesn't fix anything for WSL, it's just more code, recommend removing before merge

also:
no need to surround $(uname) in quotes, you're inside the bash builtin [[, where you're safe from spaces

It removes an unneeded sudo on macOS so it is still a net improvement. I'll remove the quotes, thanks for that tip

consider

[[ test ]] && sudo ...

?

solana-grimes · 2018-06-16T00:03:35Z

💔 Unable to automerge due to CI failure

mvines · 2018-06-16T00:09:27Z

hmm, test crdt::tests::purge_test ... FAILED with a:

---- crdt::tests::purge_test stdout ----

	thread 'crdt::tests::purge_test' panicked at 'called `Result::unwrap()` on an `Err` value: CrdtTooSmall', libcore/result.rs:945:5

note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

stack backtrace:

   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace

             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49

   1: std::sys_common::backtrace::print

             at libstd/sys_common/backtrace.rs:71

             at libstd/sys_common/backtrace.rs:59

   2: std::panicking::default_hook::{{closure}}

             at libstd/panicking.rs:207

   3: std::panicking::default_hook

             at libstd/panicking.rs:217

   4: std::panicking::rust_panic_with_hook

             at libstd/panicking.rs:402

   5: std::panicking::begin_panic_fmt

             at libstd/panicking.rs:349

   6: rust_begin_unwind

             at libstd/panicking.rs:325

   7: core::panicking::panic_fmt

             at libcore/panicking.rs:72

   8: core::result::unwrap_failed

             at /checkout/src/libcore/macros.rs:26

   9: <core::result::Result<T, E>>::unwrap

             at /checkout/src/libcore/result.rs:782

  10: solana::crdt::tests::purge_test

             at src/crdt.rs:1077

  11: solana::__test::TESTS::{{closure}}

             at src/crdt.rs:1046

  12: core::ops::function::FnOnce::call_once

             at /checkout/src/libcore/ops/function.rs:223

  13: <F as alloc::boxed::FnBox<A>>::call_box

             at libtest/lib.rs:1453

             at /checkout/src/libcore/ops/function.rs:223

             at /checkout/src/liballoc/boxed.rs:784

  14: __rust_maybe_catch_panic

             at libpanic_unwind/lib.rs:102

Retrying the build got it green on the second attempt. 🙈

* bump rollup-plugin-node-resolve from 4.2.3 to 5.0.0 * Fix NPM warnings * bump rollup-plugin-commonjs from 9.3.4 to 10.0.0

mvines requested a review from rob-solana June 15, 2018 22:37

mvines added the work in progress This isn't quite right yet label Jun 15, 2018

rwalker-com reviewed Jun 15, 2018

View reviewed changes

garious approved these changes Jun 15, 2018

View reviewed changes

mvines force-pushed the babystep branch 2 times, most recently from 3c217d8 to 24d8a8d Compare June 15, 2018 23:20

rob-solana reviewed Jun 15, 2018

View reviewed changes

mvines removed the work in progress This isn't quite right yet label Jun 15, 2018

mvines added 3 commits June 15, 2018 16:51

Skip |sudo sysctl ...| on macOS

1213863

Improve error reporting

8b9381c

Fail fast in CI when |cargo fmt| says no

7d84373

mvines force-pushed the babystep branch from 24d8a8d to 7d84373 Compare June 15, 2018 23:52

mvines added the automerge Merge this Pull Request automatically once CI passes label Jun 15, 2018

solana-grimes removed the automerge Merge this Pull Request automatically once CI passes label Jun 16, 2018

mvines merged commit 3f763f9 into solana-labs:master Jun 16, 2018

mvines deleted the babystep branch June 16, 2018 00:10

CriesofCarrots pushed a commit that referenced this pull request Feb 28, 2020

chore: rollup dependency conflict (#366)

84f267d

* bump rollup-plugin-node-resolve from 4.2.3 to 5.0.0 * Fix NPM warnings * bump rollup-plugin-commonjs from 9.3.4 to 10.0.0

mvines pushed a commit to mvines/solana that referenced this pull request Jun 15, 2020

chore: rollup dependency conflict (solana-labs#366)

b23f317

* bump rollup-plugin-node-resolve from 4.2.3 to 5.0.0 * Fix NPM warnings * bump rollup-plugin-commonjs from 9.3.4 to 10.0.0

mvines pushed a commit that referenced this pull request Jun 15, 2020

chore: rollup dependency conflict (#366)

6304814

* bump rollup-plugin-node-resolve from 4.2.3 to 5.0.0 * Fix NPM warnings * bump rollup-plugin-commonjs from 9.3.4 to 10.0.0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Various minor testnet demo script tweaks #366

Various minor testnet demo script tweaks #366

mvines commented Jun 15, 2018

mvines commented Jun 15, 2018

rwalker-com Jun 15, 2018

mvines Jun 15, 2018

rwalker-com Jun 15, 2018 •

edited

Loading

mvines Jun 15, 2018

rwalker-com Jun 15, 2018

mvines Jun 15, 2018

rwalker-com Jun 15, 2018 •

edited

Loading

mvines Jun 15, 2018 •

edited

Loading

rob-solana Jun 15, 2018

rwalker-com Jun 15, 2018

mvines Jun 15, 2018

rwalker-com commented Jun 15, 2018

rwalker-com commented Jun 15, 2018

codecov bot commented Jun 15, 2018 •

edited

Loading

rob-solana Jun 15, 2018

mvines Jun 15, 2018

rob-solana Jun 15, 2018 •

edited

Loading

mvines Jun 15, 2018

solana-grimes commented Jun 16, 2018

mvines commented Jun 16, 2018

Various minor testnet demo script tweaks #366

Various minor testnet demo script tweaks #366

Conversation

mvines commented Jun 15, 2018

mvines commented Jun 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwalker-com Jun 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwalker-com Jun 15, 2018 • edited Loading

Choose a reason for hiding this comment

mvines Jun 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwalker-com commented Jun 15, 2018

rwalker-com commented Jun 15, 2018

codecov bot commented Jun 15, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rob-solana Jun 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

solana-grimes commented Jun 16, 2018

mvines commented Jun 16, 2018

rwalker-com Jun 15, 2018 •

edited

Loading

rwalker-com Jun 15, 2018 •

edited

Loading

mvines Jun 15, 2018 •

edited

Loading

codecov bot commented Jun 15, 2018 •

edited

Loading

rob-solana Jun 15, 2018 •

edited

Loading