search

SChernykh / p2pool

Decentralized pool for Monero mining
GNU General Public License v3.0
1.08k stars 128 forks source link

HR stats cannot be reset fully right after sync's completed #154

Closed tywkah closed 2 years ago

tywkah commented 2 years ago

image image

Here are 2 examples of the latest branch. The 2nd one has a new msg 'P2POOL'S BEEN SYNCED' and was got by dropping client connections by FW after some shares had been found and stats had been updated (I'll explain why later). I tried to look into the problem and found that if I add m_cumulativeHashes = 0; to the 'void StratumServer::reset_share_counters()' section, the HR stats will indeed be reset fully: image image

BUT there may sometimes be another situation that interferes with a normal reset! Look at this screen: image We can see some "intra-" shares, the new "p2pool sync" message (that must reset all share stats) and... a share that had been found right before the sync message, which was added to the stats later when the real diff had already been too high because of the main chain sync! And another share was also found right after the sync and it even didn't have a client name and ip:port... I think there's a need in extra "low diff check" or something like this... Total hashes can NOT be reset as I suggested above (compare the hashes sum before and after the sync). And I can't even suppose what to do with the encreased total share counter value!

p.s. I've just got another example (I haven't interfered with my serv now by writin 'status' and so on): image Triple number of shares received in one second with the sync message!

SChernykh commented 2 years ago

This is 15 minute / 1 hour / 24 hour hashrate estimates, it shouldn't be reset.

SChernykh commented 2 years ago

Stale shares are a rare situation. When P2Pool is synchronized, it immediately sends new jobs to miners, so it can only happen if they find a share within 50-100 ms of the network latency. Even then, these shares are just added to the old sidechain with old difficulty, each of them has a chance to mine a Monero block, so it must be processed fully. I don't see it as a bug.

tywkah commented 2 years ago

But those stale shares increment a share and total hashes counter that must be reset in normal situation (you added this functional recently) And I wouldn't say it's a rare situation (especially at an old pc like intel atom/nm10 with 4 gb born on 2012...)!

image There's only one mainchain share in a white frame and you can see fake stats in a red one.

SChernykh commented 2 years ago

And I wouldn't say it's a rare situation (especially at an old pc like intel atom/nm10 with 4 gb born on 2012...)!

It's a rare situation for such computer to even find a share, let alone find it at the exact time p2pool gets synchronized and resets counters. Anyway, https://github.com/SChernykh/p2pool/commit/8f1f81749d164683e63063679596012bf816ff3c makes sense to do because other hash counters get reset there too.

tywkah commented 2 years ago

It's a rare situation for such computer to even find a share, let alone find it at the exact time p2pool gets synchronized

This pc's not for mining:) It can barely serve monerod+p2pool. Now it takes about 7 min to sync after start with cache after short downtime and even more w/out cache. So it lags during the syns process, ecpecially right after start and before the end of syncing. And I have to cut off miners during the sync using fw+script so that they cant interfere the stats (meanwhile they switch to another pool for that short time of sync).

8f1f817 thanks!

tywkah commented 2 years ago

image There're 34 secs between the last by one share '430' and the sync msg (meaning all stats to be reset), but it was counted in the total hashes value with the latest 'Ryzen' hashes too... And of course that one stale 'Ryzen's share incremented total number of found shares in stats. Or it can be just wrong calculation (maybe connected with 'effort' value).