Worker specific hashrate tracking extension (SOLUTION N. 3) #107
Conversation
|
if using a local MD monitor (HR temp etc) pass trought in separate channel to pool if requested can be had. connection pool for work should be a clear clean channel . statistics another ? Q : is request to be able monitor MD local to pool or do pool want statistics of remote HR connections . Why ... |
| |---------------|------------------|---------------------------------------| | ||
| | channel_id | U32 | Channel identifier | | ||
| | request_id | U32 | Unique request ID | | ||
| | user_id | SEQ0_64K[U32] | Unique IDs for each worker | |
There was a problem hiding this comment.
This would mean that this extension would support a maximum of 65536 machines. Do you think it's suitable also for very large operations?
There was a problem hiding this comment.
Is this across an entire pool or just a farm level consideration? either way, 65536 is likely insufficient for large farms.
There was a problem hiding this comment.
If 64k isn't enough then we need to switch to the version that just includes the full name. Making the pool store more than 64k entries in a hashmap for each connection isn't really a great option IMO.
There was a problem hiding this comment.
Making the pool store more than 64k entries in a hashmap for each connection isn't really a great option IMO.
Why do you think this? Isn't it what pools are already doing more or less since they receive shares from every worker?
There was a problem hiding this comment.
@Fi3 would you prefer to set a fixed predetermined bytes limit, of a more flexible way such the one I was proposing? PS: in the meantime I'm going to update option n.1 on doc to make it an extension
There was a problem hiding this comment.
I modified option n.1 on doc, and opened a new PR to implement it as an extension: #108
There was a problem hiding this comment.
I don't see why adding new fields at the end of existing messages should break existing clients/servers. Those fields should simply be ignored by existing code. If not, we really need to define a way to add fields to existing messages without wrecking compatibility because that's kinda important :)
There was a problem hiding this comment.
I initially defined solution n.1 without extensions in the doc, but feedback got from @Fi3 made me change it to an extension.
There was a problem hiding this comment.
I don't see why adding new fields at the end of existing messages should break existing clients/servers. Those fields should simply be ignored by existing code. If not, we really need to define a way to add fields to existing messages without wrecking compatibility because that's kinda important :)
Adding field to message will break the parser, I don't see how we can have optional fields without changing the binary format. Maybe we can add to each message a field called optional data that is an B016M, but it seems pretty crazy to me. I think that having extensions + sv2 versions will be enough.
The point you're making is very similar to point n.4 of the linked document, we are going to get there in the future for sure. |
|
yes but is pool level local or remote ? why would pool want spend BW on singular MD at several PH remote farm ? maybe You are shouting yourself in foot by trying bend over for stupid metrics ? i asked . do Your requests need stats for remote worker to pool . Why ... |
|
The pool is remote of course. Nowadays they are spending the same bandwidth (actually more) than what is proposed in this extension (because of the |
|
so for ex. a farm proxy aggregating to a pool say braiins farm proxy . i do not think they by default try measure farm individual workers (MD) . why in that case try have a farm proxy = aggregate hashers locally has need for statistics . pool do not ! ... |
Hashers often rely on pool for statistics via watcher links and whatnot. |
|
in a pool dashboard a proxy aggregated connection would look like one workers HR in hashers local proxy "dashboard" one should be able se aggregated HR to upstream in hashers local net, hasher have stats of workers dashboards before aggregated to a proxy upstream a pool only should be concerned with a worker connected (aggregated or not) share submissions at difficulty be able fairly compensate work a pool with remote workers should not have to have any concern of single workers behind a proxy performing at a HR |
|
We've talked to several pools and some expressed the need for this, others didn't that's why it's an extension. Let's try to keep discussion focused on the topic, not if the pool needs this or not, because we've done our fair share of researching and it's a need of at least 2 early adopters implementing SV2 in their pool. This isn't just the dashboard issue, certain pools need it for regulatory reasons. |
|
Would be better to put extensions either in their specific repo or in separate directory (for extension) within this repo. |
|
|
||
| ### 1.1 Activate Extension | ||
|
|
||
| A client wishing to use this extension sends the `Activate` message. If supported, the pool responds with `Activate.Success`. Otherwise, the client stops further attempts. |
There was a problem hiding this comment.
This should be a global and not extension specific. Every extension have to be activated with the same procedure. Beeing this one or another one. @jakubtrnka
| | request_id | U32 | Original request ID | | ||
| | user_id | SEQ0_64K[U32]| Worker ID associated | | ||
|
|
||
| ### `SetUserIdentity.Error` (Server -> Client) |
There was a problem hiding this comment.
What the client do now? I guess only possible thing is close the connection, since there is no procedure to recover from this error. I belive that having specific error message for irrecoverable situation is only cause of confusion. IMO is much better having a general error with a string that we use only for logging. In that case the pool send the error and close the connection, then downstream can log what caused the error.
If there is a way to recover we should have a set of messages to actually do it.
There was a problem hiding this comment.
Here the pool should close the connection because it means something is not working properly or client is trying to spam the server. Does it make sense?
GitGab19
force-pushed
the
set-user-identity
branch
from
5fc80c6
to
27d1751
Compare
I already did it. The file is inside a new |
GitGab19
changed the title
As described in this doc, this extension is the solution to a specific request we got from several early adopters.
It allows to get workers granularity on shares submitted through
extended-channels, without losing the aggregation advantages of them.I described two possible approaches for changes that touch the
SubmitSharesExtendedmessage. I would really appreciate any feedback on this, so that we can pick up the best option.