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

dublicate job #5

Closed
1rV1N-git opened this issue Jan 5, 2018 · 21 comments
Closed

dublicate job #5

1rV1N-git opened this issue Jan 5, 2018 · 21 comments

Comments

@1rV1N-git
Copy link

1rV1N-git commented Jan 5, 2018

Hello i think there is need to add code to prevent send duplicate job.

function newBlockTemplate(template) {
let buffer = new Buffer(template.blocktemplate_blob, 'hex');
let previous_hash = new Buffer(32);
buffer.copy(previous_hash, 0, 7, 39);
console.log(threadName + 'New block to mine at height: ' + template.height + '. Difficulty: ' + template.difficulty);
if (activeBlockTemplate) {
if (activeBlockTemplate.previous_hash.toString('hex') === template.prev_hash ) {
console.log(threadName + "Same job");
return;
}

pastBlockTemplates.enq(activeBlockTemplate);
}
activeBlockTemplate = new BlockTemplate(template);
console.log(threadName + "Update workers");
for (let minerId in activeMiners) {
if (activeMiners.hasOwnProperty(minerId)) {
let miner = activeMiners[minerId];
debug(threadName + "Updating worker " + miner.payout + " With new work at height: " + template.height);
miner.sendNewJob();
}
}
}

example:
1|pool | 2018-01-05 21:18:22 +00:00: (Worker 4 - 63492) New block to mine at height: 42585. Difficulty: 75447651
1|pool | 2018-01-05 21:18:22 +00:00: (Worker 4 - 63492) activeBlockTemplate : 1cd23e23e48d238c57e536a8aac2817220ee6be70fad8b5b3c79279ae8515d09
1|pool | (Worker 4 - 63492) 7062b336bc841d0d0dea6b09e94bf3b3d42371019252dbf1dbd8fdaff5af2140
1|pool | (Worker 4 - 63492) Update workers
1|pool | 2018-01-05 21:18:22 +00:00: (Worker 1 - 63474) New block to mine at height: 42585. Difficulty: 75447651
1|pool | 2018-01-05 21:18:22 +00:00: (Worker 1 - 63474) activeBlockTemplate : 1cd23e23e48d238c57e536a8aac2817220ee6be70fad8b5b3c79279ae8515d09
1|pool | 2018-01-05 21:18:22 +00:00: (Worker 1 - 63474) 7062b336bc841d0d0dea6b09e94bf3b3d42371019252dbf1dbd8fdaff5af2140
1|pool | 2018-01-05 21:18:22 +00:00: (Worker 1 - 63474) Update workers
1|pool | 2018-01-05 21:18:22 +00:00: (Worker 2 - 63480) New block to mine at height: 42585. Difficulty: 75447651
1|pool | (Worker 2 - 63480) activeBlockTemplate : 1cd23e23e48d238c57e536a8aac2817220ee6be70fad8b5b3c79279ae8515d09
1|pool | (Worker 2 - 63480) 7062b336bc841d0d0dea6b09e94bf3b3d42371019252dbf1dbd8fdaff5af2140
1|pool | (Worker 2 - 63480) Update workers
1|pool | 2018-01-05 21:18:22 +00:00: (Worker 3 - 63486) New block to mine at height: 42585. Difficulty: 75447651
1|pool | 2018-01-05 21:18:22 +00:00: (Worker 3 - 63486) activeBlockTemplate : 1cd23e23e48d238c57e536a8aac2817220ee6be70fad8b5b3c79279ae8515d09
1|pool | (Worker 3 - 63486) 7062b336bc841d0d0dea6b09e94bf3b3d42371019252dbf1dbd8fdaff5af2140
1|pool | (Worker 3 - 63486) Update workers
1|pool | 2018-01-05 21:18:22 +00:00: (Master) New block to mine at height: 42585. Difficulty: 75447651
1|pool | 2018-01-05 21:18:22 +00:00: (Master) activeBlockTemplate : 1cd23e23e48d238c57e536a8aac2817220ee6be70fad8b5b3c79279ae8515d09
1|pool | (Master) 7062b336bc841d0d0dea6b09e94bf3b3d42371019252dbf1dbd8fdaff5af2140
1|pool | 2018-01-05 21:18:22 +00:00: (Worker 4 - 63492) New block to mine at height: 42585. Difficulty: 75447651
1|pool | (Worker 4 - 63492) activeBlockTemplate : 7062b336bc841d0d0dea6b09e94bf3b3d42371019252dbf1dbd8fdaff5af2140
1|pool | (Worker 4 - 63492) 7062b336bc841d0d0dea6b09e94bf3b3d42371019252dbf1dbd8fdaff5af2140
1|pool | (Worker 4 - 63492) Same job
1|pool | 2018-01-05 21:18:22 +00:00: (Master) Update workers

@MoneroOcean
Copy link
Owner

OK. I never looked into this code, so it will take time for me to completely understand what is going on here and if fix is helpful. If not yet already done I recommend to submit same issue against main repo too.

@MoneroOcean
Copy link
Owner

MoneroOcean commented Jan 6, 2018

Actually I do not think it is a problem here and moreover fix will lead to harmful effect. Fix rejects new block template if hash of previous block is the same as hash of previous block of active block template, right? This is not right, since monerod can provide updated valid block templates with the same previous block hash (for example if height is not changed and there are new transaction included into block template). This fix will merely allow the first block template to be used for each height and will reject their valid updates that can lead to lower block rewards (because less tx fees are included).

Hope it make sense. Feel free to point me if I'm wrong here.

@1rV1N-git
Copy link
Author

Ok, I understood you. I did not know it could be that way.
There you can compare by blocktemplate_blob or maybe it's worth it just to put a temporary restriction, for example in 2 seconds.

@1rV1N-git
Copy link
Author

1rV1N-git commented Jan 6, 2018

I want to prevent this
image

@bobbieltd
Copy link

@1rV1N-git : Snipa’s repo got same bug like yours before
See Snipa22#198
@MoneroOcean Did you merge the “Duplicate Job” fix from Snipa’s repo ?

@MoneroOcean
Copy link
Owner

MoneroOcean commented Jan 6, 2018

Yeah, sure. It was merged on the same day: 6a08322
Also, as I understood from the picture from port number (3333) it is some generic nodejs-pool issue, not specific to MO repo (and it is different from Snipa22#198 since here it is xmrig who complains that he got the same job from pool again).
Like I said I'm not very fluent in this part of pool's code so need more time.

@bobbieltd
Copy link

bobbieltd commented Jan 6, 2018

I got same error "Duplicate jobs" multiple time and Snipa did fix it, no more that errors after that fix. I've never see that errors again from that day.
@MoneroOcean You didn't merge "Fix for the 300 msec template bug identified by Mayday30."
https://github.com/MoneroOcean/nodejs-pool/blob/master/lib/pool.js#L174 doesn't reflect fix :
} else {
if (repeating !== true) {
setTimeout(templateUpdate, 300);
}
}
});
}
...........

@MoneroOcean
Copy link
Owner

MoneroOcean commented Jan 6, 2018

This particular line/fix you mentioned is not related to duplicate jobs. It is fix for frequent block template updates and I decided to rework it after (that is why this line is different). I'm pretty sure in this particular case since actually I'm Mayday30;)

Also I'd like to reiterate that according to port number (we do not host 3333 port) this is not MO specific issue (not surprising since I never touched job generation code). Please correct me if I'm wrong.

@1rV1N-git
Copy link
Author

I use snipa repo wtih Mayday30 fix in Almost all nodes c addition from your repo becase port 3333))
It is so strange becase the screenshot that i send it is pool node by your repo.
I added this code and I got in log "Same job".
You may add the code without return for debag the error.
In a couple of days I plan to change all node to your repo.

@bobbieltd
Copy link

bobbieltd commented Jan 6, 2018

Update 1 : I did get error from newly migrated MO repo pool. I'll run test with moneroocean.stream now, update 2 in 20 minutes.
duplicate jobs

Update 2 : Test on MO pool, port 10001 is fine. No errors. I run last test on my pool with port 5555 and give results in Update 3.

Update 3 : Got error with port 5555. I come back to test one more time with moneroocean pool because in previours test, I did see "no active pools, stop mining" but not "duplicate job" so I thought because of my slow PC. One more test to check out. Result on MO pool will be in update 4.
duplicate jobs 2

Update 4 : moneroocean.stream works fine. I don’t get any “duplicate job”. I’ll try to test other Snipa pools again.

@1rV1N-git
Copy link
Author

This error does not appear so often during normal operation. it always appears when the pool is restarted and when the block is found. the rest of the time one for 30-50 updates of the block template

@MoneroOcean
Copy link
Owner

That is very useful statistics and will help me to narrow down the problem area. But why do not you submit that against main Snipa's repo? I will try to investigate and fix that for course but I think he can do that much more easier (not sure if faster:).

@1rV1N-git
Copy link
Author

(not sure if faster:) that is why )

@MoneroOcean
Copy link
Owner

Looks like I finally managed to get better picture what is going on here. You got the problem right and I was wrong in my first comment about the fact that block template can be updated over time when prev_hash is not changed. Looks like this situation can happen when thread BT update happens before rapid BT update from main thread. In this case newBlockTemplate(...) is called twice (first from worker thread and second from main thread during sendToWorkers({type: 'newBlockTemplate', data: rpcResponse});).

@MoneroOcean
Copy link
Owner

The fix will be very similar to what you suggested.

@MoneroOcean
Copy link
Owner

This should fix this:

1e8d0a0
29b6fd7
ac4ea5c

@bobbieltd
Copy link

Thanks. This fix concerns pool.js and xmr.js , if I want to apply the fix, what do I need to do ?
pm2 stop all
cd ~/nodejs-pool
git pull
pm2 start all
Is that process correct ?

@MoneroOcean
Copy link
Owner

For that fix you only need to restart pool module:

cd ~/nodejs-pool && git pull && pm2 restart pool
pm2 logs pool

Will do the trick.

@MoneroOcean
Copy link
Owner

xmr.js fixes are not necessary to fix this particular issue but it is related bug fixes.

@1rV1N-git
Copy link
Author

Excellent Job!!. Thanks a lot

@bobbieltd
Copy link

Thank you.

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

3 participants