[CO-390] Run Demo Cluster #3629

KtorZ · 2018-09-19T10:10:40Z

Description

@disassembler, you'll recognize some parts of an old PR. I've resurrected it and adjusted a few things:

Now we pick a system temporary directory as a state-dir (unless given one)
Topology is generated on the fly
TLS certs are generated on the fly

This is still WIP but I'd appreciate your inputs before I move forward ❤️

Linked issue

[CO-390]

Type of change

🐞 Bug fix (non-breaking change which fixes an issue)
🛠 New feature (non-breaking change which adds functionality)
⚠️ Breaking change (fix or feature that would cause existing functionality to change)
🏭 Refactoring that does not change existing functionality but does improve things like code readability, structure etc
🔨 New or improved tests for existing code
⛑ git-flow chore (backport, hotfix, etc)

Developer checklist

I have read the style guide document, and my code follows the code style of this project.
If my code deals with exceptions, it follows the guidelines.
I have updated any documentation accordingly, if needed. Documentation changes can be reflected in opening a PR on cardanodocs.com, amending the inline Haddock comments, any relevant README file or one of the document listed in the docs directory.
CHANGELOG entry has been added and is linked to the correct PR on GitHub.

Testing checklist

I have added tests to cover my changes.
All new and existing tests passed.

QA Steps

Screenshots (if available)

KtorZ · 2018-09-19T10:11:47Z

NOTE (@disassembler): I haven't tested anything yet so don't try to run it ^^" ... It's more about grooming the general idea.

CodiePP · 2018-09-19T10:55:53Z

wallet-new/src/Cardano/Wallet/Demo.hs

+know:
+
+- ENV variables define "base" arguments for a node.
+  When it makes sense (.e.g `DB_PATH`, `LOG_CONFIG`), these "base" arguments


will you also support a prefix where logs are put?
--logs-prefix FILEPATH Prefix to logger output path.

Since it reuses the CLI parsers, everything that is supported by the CLI will be seemingly supported by this. 👍

KtorZ · 2018-09-19T13:18:50Z

Just realised there's no need to run most of those actions in plain IO. The ENV can be read once and put in a Map, then we can use normal lenses to perform the same operation in a pure fashion.

Will do.

KtorZ · 2018-09-25T16:16:35Z

lib/src/Pos/Launcher/Launcher.hs

+        genesisConfig
+        txpConfig
+        nodeRes
+        (runNode genesisConfig txpConfig nodeRes plugins)


Moved from node/Main.hs to be re-used elsewhere.

jmitchell · 2018-10-11T20:10:01Z

cluster/README.md

+......system start:  1539179292
+......address:       127.0.0.1:3100
+...wallet OK!
+......system start:  1539179292


@disassembler tells me the inconsistent start times have been fixed. Let's update the example text here too.

jmitchell

Only a minor doc nitpick, but otherwise LGTM.

This code makes use of the 'Cardano.Wallet.Demo' module and show how it can be used to spawn a demo cluster of nodes. More details in the README.md

KtorZ · 2018-10-16T12:47:04Z

Still waiting for review from @akegalj, though this is needed for debugging and assessing results for a fix of 1.4.0.
It's quite isolated work so we can simply iterate with @akegalj 's feedback (if any).

akegalj

LGTM

akegalj · 2018-10-16T14:46:44Z

cluster/src/Cardano/Cluster.hs

+                    yield (RunningRelayNode nodeId nodeEnv)
+
+                NodeEdge -> do
+                    genesisKeys <- init topology >> init logger >> init genesis


I like how this makes it clear what options are being included 👍

akegalj · 2018-10-16T14:54:39Z

cluster/src/Cardano/Cluster.hs

+    -> (Maybe SyncPercentage -> IO ())
+    -> IO ()
+waitForNode wc (MaxWaitingTime s) reportProgress = do
+    res <- race (threadDelay $ s * oneSecond) retry


akegalj · 2018-10-19T18:16:32Z

cluster/src/Cardano/Cluster/Environment.hs

+    :: NodeType                               -- ^ Target node type (core, relay, edge ...)
+    -> [(NodeName, NodeType, NetworkAddress)] -- ^ All fully qualified nodes
+    -> Topology
+demoTopology nodeType =


I have just skimmed over this one as I am lacking more indepth knowledge about the internals of topology

akegalj · 2018-10-19T18:30:19Z

cluster/src/Cardano/Cluster/Util.hs

+    -- terminating "normally" which can't happen.
+    mvar   <- newEmptyMVar
+    handle <- async (action (putMVar mvar))
+    result <- race (wait handle) (takeMVar mvar)


akegalj · 2018-10-19T18:38:12Z

cluster/src/Cardano/Cluster/Util.hs

+sToK = map (Char.toLower . replaceIf '_' '-')
+
+
+-- | Split a string on the given character


I wonder if you could have reused split or splitOn from Text: http://hackage.haskell.org/package/text-1.2.3.1/docs/Data-Text.html#v:splitOn , http://hackage.haskell.org/package/text-1.2.3.1/docs/Data-Text.html#v:split

akegalj · 2018-10-20T07:39:09Z

cluster/test/Cardano/Cluster/Util/Spec.hs

+prop_runAsync n = ioProperty $ do
+    ((handle, res), t) <- time (runAsync io)
+    cancel handle
+    return $ classify (t < 100) "< 100μs" $ res === n .&&. t < (10 * oneSecond)


I wonder will this sometimes fail (with low probability) if thread scheduler paused async action

Probably not. I took a veeeeery large time for the property to fail (10 seconds). That's also what the classify is for. I've actually seen some runs where 99.9% only are under 100us and some got above. The property still passes fine as it's not about measuring any time execution, but just measuring that yields correctly returns a result prematurely, before the end of the thread.

KtorZ requested a review from disassembler September 19, 2018 10:10

CodiePP reviewed Sep 19, 2018

View reviewed changes

KtorZ force-pushed the KtorZ/CO-390/run-demo-cluster branch 3 times, most recently from cca17c9 to 337b08f Compare September 25, 2018 16:15

KtorZ commented Sep 25, 2018

View reviewed changes

KtorZ added the wip label Sep 25, 2018

KtorZ requested a review from adinapoli-iohk September 25, 2018 16:29

KtorZ force-pushed the KtorZ/CO-390/run-demo-cluster branch from 337b08f to f30a720 Compare September 28, 2018 12:04

KtorZ removed the wip label Sep 28, 2018

KtorZ changed the title ~~[CO-390] WIP - Run Demo Cluster~~ [CO-390] Run Demo Cluster Sep 28, 2018

KtorZ force-pushed the KtorZ/CO-390/run-demo-cluster branch 6 times, most recently from 4d71034 to 963ec1e Compare October 5, 2018 08:11

KtorZ force-pushed the KtorZ/CO-390/run-demo-cluster branch 8 times, most recently from 73333d0 to be213ad Compare October 10, 2018 16:31

disassembler approved these changes Oct 10, 2018

View reviewed changes

jmitchell reviewed Oct 11, 2018

View reviewed changes

cleverca22 approved these changes Oct 11, 2018

View reviewed changes

jmitchell approved these changes Oct 11, 2018

View reviewed changes

KtorZ added 3 commits October 12, 2018 10:11

[CO-390] Introduce library code for running a demo cluster

64f3660

[CO-390] Create a 'demo' executable for cardano-sl-wallet-new

0dc0442

This code makes use of the 'Cardano.Wallet.Demo' module and show how it can be used to spawn a demo cluster of nodes. More details in the README.md

[CO-390] Write a few property-based test for cluster's utils

8b76d29

KtorZ force-pushed the KtorZ/CO-390/run-demo-cluster branch from e6b18c3 to 8b76d29 Compare October 12, 2018 08:11

KtorZ merged commit 8e26a7a into develop Oct 16, 2018

KtorZ deleted the KtorZ/CO-390/run-demo-cluster branch October 16, 2018 12:47

akegalj reviewed Oct 20, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CO-390] Run Demo Cluster #3629

[CO-390] Run Demo Cluster #3629

KtorZ commented Sep 19, 2018

KtorZ commented Sep 19, 2018

CodiePP Sep 19, 2018

KtorZ Sep 19, 2018

KtorZ commented Sep 19, 2018

KtorZ Sep 25, 2018

jmitchell Oct 11, 2018

jmitchell left a comment

KtorZ commented Oct 16, 2018

akegalj left a comment

akegalj Oct 16, 2018

akegalj Oct 16, 2018

akegalj Oct 19, 2018

akegalj Oct 19, 2018

akegalj Oct 19, 2018

akegalj Oct 20, 2018

KtorZ Oct 25, 2018

		sToK = map (Char.toLower . replaceIf '_' '-')


		-- \| Split a string on the given character

[CO-390] Run Demo Cluster #3629

[CO-390] Run Demo Cluster #3629

Conversation

KtorZ commented Sep 19, 2018

Description

Linked issue

Type of change

Developer checklist

Testing checklist

QA Steps

Screenshots (if available)

KtorZ commented Sep 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KtorZ commented Sep 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmitchell left a comment

Choose a reason for hiding this comment

KtorZ commented Oct 16, 2018

akegalj left a comment

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

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