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

distributed architecture and preparation for higher load pt. 2 #132

Closed
mpnowacki-reef opened this issue Jul 5, 2021 · 11 comments
Closed

Comments

@mpnowacki-reef
Copy link
Contributor

Hey,

In relation to #91 we would now like to introduce the possibility to start the pool-reference server in multiple instances (probably behind a reverse proxy). In order to achieve that, several looped tasks would have to be factored out from the Pool class, namely:
create_payment_loop
submit_payment_loop
collect_pool_rewards_loop
The reason for that is that multiple instances would inevitably try to "spend the same coin". And while they would not succeed, at least eventually, this would cause a lot of unnecessary CPU usage and warning logs.
The loops mentioned above would be moved to a class named PaymentManager. An instance of PaymentManager would be supplied at PoolServer creation, with the implementation functionally identical to the current one serving as default. (this is very similar to the changes I introduced in #99)

The Idea we have in mind is that we would like to start N PoolServer instances and just one PaymentManager doing all the cron-like work (the N instances and the separate PaymentManager would use the same database).

My question is: would a PR introducing such refactor be welcome?

@mariano54
Copy link
Contributor

Yes it would be welcome, as long as it does not harm the operation of the pool in the default (single instance) configuration. Thanks!

@mpnowacki-reef
Copy link
Contributor Author

I'm almost done with this one, I do have one question though: right now, in submit_payment_loop in each iteration there a log_in_and_skip call performed. Is this necessary? Should I leave it as it is? We know the credentials we have are good, because the same call is performed in Pool.start.

@mariano54
Copy link
Contributor

It's necessary if you have multiple wallets running on that machine. Otherwise, it will just return without doing anything.
Therefore, I don't see a harm in leaving that in there.

@mpnowacki-reef
Copy link
Contributor Author

okay, i'll do that

@ppolewicz
Copy link
Contributor

@mariano54 we are committed to keeping the backward compatibility for single instance pools. At the same time, I believe it is possible to make the reference pool easier to adjust, both for when it's running on a single node or not. Rest assured, if at some point a major change would require breaking the compatibility, we would surely have a bigger discussion in issues about it to get your go-ahead before coding it.

@mariano54
Copy link
Contributor

Thanks @ppolewicz, sounds good!

@alvaroslm
Copy link

A "reference" pool certainly doesn't need this. It just complicates things, makes code less clear and ultimately solves nothing by itself. Either make it simple or make it a full-fledged production pool. But muddying the waters with something in-between helps no one.

@jack60612
Copy link
Contributor

agreed @alvaroslm

@ppolewicz
Copy link
Contributor

We are trying to iteratively increase the quality of the reference pool with a very sharp goal in mind. I think it would be good if the reference pool, or at least major parts of it, would be used by commercial pool operators. We are trying to make the pool code more modular, so that it's more customizable and also more readable.

As #142 shows, this refactor moves a large chunk of functionality from pool.py, which I think makes the project easier to read. If perfect readability would be achieved by having a minimum number of classes in a project, everything would be in one file and one class, but that's not how SOLID works.

As for helping nobody, I strongly disagree - those changes will help the pool operators who choose to run their pools on top of a customized reference pool implementation (which reduces the cost of implementing a pool by a lot). Not in some distant future, but this week.

@alvaroslm
Copy link

@ppolewicz then fork it and stop tainting this repo. "SOLID" is dangerously misguided if followed to the letter and represents an obsessive-compulsive engineer mindset. It's like having a desk with 200 drawers, one for each item you have on the table. Yeah you keep the table clean, at the cost of becoming insane. 21200 10-line functions, hierarchies 25 levels deep... sheer insanity. Obsessive modularity destroys software projects. It looks good on textbook examples, in the real world, not so much.

@mariano54
Copy link
Contributor

PRs are welcome and helpful. Each PR is reviewed and considered independently.

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

5 participants