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

It should be possible to start multiple instances of ElectrumSV for an SDK run #16

Closed
rt121212121 opened this issue Aug 17, 2020 · 19 comments

Comments

@rt121212121
Copy link
Contributor

  • Maybe something like electrumsv-sdk start electrumsv --gui.
  • All wallet data directories should be stored under the sdk data directory (now in AppData\Local\ElectrumSV-SDK or /home/.electrumsv-sdk).
  • Each instance run should have a user-addressable name, whether the user provides it or it is allocated if none is given. This means that if they close a specific instance with it's wallet files and state, they should be able to start that instance again.
@AustEcon
Copy link
Contributor

AustEcon commented Aug 19, 2020

I think I should change the CLI structure for the "start" command.

Instead of:
electrumsv-sdk start --esv-ex-node

to signify starting ElectrumSV wallet x 1 + ElectrumX x 1 + Node x 1

it should be:
electrumsv-sdk start --node --ex --esv-gui --esv-daemon --esv-daemon --esv-daemon "3rd_daemon"

or something like that which would start:

  • 1 x Node
  • 1 x ElectrumX
  • 1 x ElectrumSV GUI instance
  • 3 x ElectrumSV daemon instances where the "3rd_daemon" gets a user-defined identifier. (the other ElectrumSV instances would be auto-allocated generic identifiers...

Then these identifiers could be used by the stop command for granular control over re-starting or stopping specified wallets.

I think we'd want there to be a 1:1 relationship between a wallet database and a given identifier right?

edit: (1 second after you already commented)

The thing I'd need to think through a bit is...
if the user defines an ID in the CLI that exists, it should re-start that same wallet (and wallet database).
if the user defines an ID in the CLI that does not exists, it should create a new wallet (and wallet database).
if the user does not specify any ID in the CLI, it should auto-allocate (e.g. "worker1")
BUT should auto-allocation ALWAYS trigger creating a new wallet? I suspect the answer is yes...
And so the only possible way to re-start the same esv-daemon with the same wallet database is to specify either the user-defined or previously auto-allocated ID.

@rt121212121
Copy link
Contributor Author

Can we rethink this and make it a bit different.

My preference is that we should only allow one start component in one command. So in theory, we should be able to go:
electrumsv-sdk start node --some-option=1
electrumsv-sdk start electrumsv --some-option=2

Starting a aggregate component collection should not necessarily allow much if any customisation:
electrumsv-sdk start legacy-stack

Then to start the GUI alongside whatever is there should be another command:
electrumsv-sdk start electrumsv --gui --id=gui-1

@rt121212121
Copy link
Contributor Author

I would lean towards no id meaning a default id, and the user can only start one instance. The user would have to explicitly give instruction to start a new instance that wasn't the default one for a component type.

@rt121212121
Copy link
Contributor Author

Which leads me to this: electrumsv-sdk start --new --gui --id=gui-1 electrumsv --option-for-esv

Or electrumsv-sdk start <start options> <component name> <options to pass to component>

@AustEcon
Copy link
Contributor

AustEcon commented Aug 19, 2020

I think that sounds okay except that I would move --gui out of <start options> and into <options to pass to component> because electrumx for example doesn't have a gui so the model doesn't "fit"

@AustEcon
Copy link
Contributor

AustEcon commented Aug 19, 2020

I'm also unsure if it would be good to include the "--new" flag for electrumx and the node for example because now would that mean that we need to maintain multiple directories of electrumx and node database / state... that would get hella complicated...

So maybe both "--gui" and "--new" would need to go into the <options to pass to component> category and be supported when it makes sense for the given component.

And further to this point... if we are not supporting "--new" for a component, the "--id" flag kind of loses relevance to an extent as well... but maybe we would make it so that we can only have 1 instance of electrumX and node database and so --id would act to rename the existing id to a new id?

I think you had it better the first time with the more simple:
electrumsv-sdk start <component name> <options to pass to component>

because these flags require the context of component type

@rt121212121
Copy link
Contributor Author

Then you're mixing options to pass to the component and sdk options for starting the component.

@rt121212121
Copy link
Contributor Author

We will have multiple instances of both nodes and indexers. In fact it will be necessary for some testing. And yes, a component should know it's data directory location. If a component does not have a gui mode then it would error.

@AustEcon
Copy link
Contributor

AustEcon commented Aug 19, 2020

Oh so I have misunderstood the meaning of <options to pass to component> - by that you mean standard built-in cli of electrumsv... you want to expose access to all of that here right?

In that case... I guess it's okay but I think if we do it via:
electrumsv-sdk start <start options> <component name> <options to pass to component>

we should enforce that --new and --gui result in an error message for ElectrumX and the Node to say that these flags are not supported for these component types... otherwise -> too complex.

edit: I posted this 1 second after your last comment haha...

Sure. Okay. I think this sounds okay

@rt121212121
Copy link
Contributor Author

I think we can make it non-complex. Every component type should have a default instance mode and declare how it responds to component level options, like new and gui. But they should all be done generically so it provides for future multiple instancing when we have time for those bonus tasks.

@rt121212121
Copy link
Contributor Author

Here's one option for gui mode and non-gui mode for the node.

  • Gui mode opens a wrapper application that opens a dos window that launches the node in the background and provides a tail -f view of bitcoind.log. Or maybe it provides a web server that has a web page that provides the same - or maybe it is possible to optionally set a value for --gui that overrides the former and does the latter --gui=web versus the default --gui that is the same as --gui=console.
  • Non-gui mode just does what it does now with no console window and running in the background.

@rt121212121
Copy link
Contributor Author

I am not sure regtest can do multiple nodes and don't think there's any benefit I can see. But multiple instances of electrumx, definitely need that.

@rt121212121
Copy link
Contributor Author

Maybe the web-gui for the node is overkill, and the dashboard should just integrate it. Then the console version might be enough.

@AustEcon
Copy link
Contributor

AustEcon commented Aug 19, 2020

Sure. I wish there were a way to have the component name to the left of the options. It's not very intuitive to me otherwise...
especially if you have a whole lot of options and then far off in the distance to the right side of screen... you eventually find the component name in there... which finally gives the whole thing context

Does:
electrumsv-sdk start <component name> <start options> --component-cli <options to pass to component>
work for you?

Where <start options> includes:

  • --repo (url or local file path)
  • --branch (of git repo)
  • --new
  • --gui
  • --component-cli

@AustEcon
Copy link
Contributor

Yes. I think in the short term we can just disable the --gui option with an error message explaining... and can wire something up later when we have the time.

@rt121212121
Copy link
Contributor Author

No. That does not work for me. Think of it like pip install --user -r requirements.txt, --user and -r do not come after requirements.txt.

@rt121212121
Copy link
Contributor Author

I want us to go for explicit and standard style approaches over custom versions that suit our preferences.

@AustEcon
Copy link
Contributor

AustEcon commented Aug 19, 2020

That's fine. I was more thinking of how the systemd / systemctl model does things and how (thinking of English grammar) usually people are conditioned to expect a sentence to lead with the subject before the predicate ("the people are walking" instead of "walking is what the people were doing". (like Yoda 🤣 ).

But it's not so bad. Sounds like a plan.

PS: I am not sure how the passing of arguments to the underlying electrumsv will work off the top of my head because I already pass quite a few arguments in the generated .bat or .sh files... but I suppose can make an adapter in the middle to make it work somehow... discussion for when we get to it.

AustEcon added a commit that referenced this issue Sep 1, 2020
…g of argument parsing and acceptance of only a single component for starting or stopping...

- added a facade for the electrumsv CLI
- always start status monitor if not already running.
- silence connection error when checking if status monitor is running.
- no component argument will start or stop all active components
@AustEcon
Copy link
Contributor

AustEcon commented Sep 1, 2020

pushed 3 commits that largely address this re-organisation of the CLI. But still requires some 'massaging'.

reminders to self:

  • --new and --gui startup flags are captured by the cli interface but do not actually modify behaviour yet
  • --id captures a human-readable identifier and stores it in component_state.json but is not yet wired up for killing processes based on this id.
  • electrumsv-sdk stop electrumsv stops based on component type - I think killing by component type should remain supported

@AustEcon AustEcon closed this as completed Oct 2, 2020
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

No branches or pull requests

2 participants