< bitcoin-git>
[bitcoin] fanquake closed pull request #9405: Contrib: Mac: Fix mac deploy so that the dmg background works (master...macPythonTweak) https://github.com/bitcoin/bitcoin/pull/9405
< jonasschnelli>
luke-jr: you wrote "If user aborts at T=5, and doesn't shut down the node, when it does finally shutdown at T=5000,"....
< jonasschnelli>
The user can only abort over a shutdown, right?
< luke-jr>
jonasschnelli: right, read the rest :p
< jonasschnelli>
IMO a "DoNotSave" and "PleaseSave" state should not be part of LoadMempool()
< jonasschnelli>
It should return what happend.
< luke-jr>
ok
< jonasschnelli>
the decision wether to dump or not should be part for DumpMempool
< jonasschnelli>
(of the controlling code)
< luke-jr>
then perhaps change user aborted to PartiallyLoaded?
< luke-jr>
although that's not much different I guess
< jonasschnelli>
Yes. Why not.
< jonasschnelli>
Though, it can only happens if the user has aborted. :)
< luke-jr>
or if the software decides to shutdown for any other reason (corrupt db after initial check?) I guess
< jonasschnelli>
but: user abort = don't dump, corrupt db = dump
< jonasschnelli>
otherwise you overwrite an intact dump
< jonasschnelli>
or you don't overwrite an invalid/corrupted dump
< luke-jr>
not necessarily a problem? just treat it same as when the user requests it
< jonasschnelli>
But wait... if you example happens, the shutdown would result in userAborted which would not dump the mempool,... seems to be okay? Not?
< sipa>
how about returning a boolean "loaded everything that could be loaded"
< paveljanik>
sipa, +1
< sipa>
a corrupted mempool.dat file is no different than an empty onr
< sipa>
from the application's perspective
< jonasschnelli>
true = loaded successfull or corrupted, false = user aborted?
< sipa>
yes
< sipa>
but does that need a return state at all?
< jonasschnelli>
The true in case of a corrupted mempool seems strange.
< sipa>
doesn't the caller also know that an abort happened?
< jonasschnelli>
Yes. fShutdownRequested.
< sipa>
so, don't dump in case of fShutdownRequested
< jonasschnelli>
true
< sipa>
immediately after LoadMempool
< jonasschnelli>
Okay. Let me try to change this.
< jonasschnelli>
sipa: but fRequestShutdown is always true during DumpMempool(), not?
< jonasschnelli>
Hmm... tried a bit, but, return true in case of "failed to open mempool" results in code hard to read. I guess I prefer my current solution.
< sipa>
and at shutdown { ...; if (fDumpMempoolLater) DumpMempool(); ... }
< jonasschnelli>
Okay. This make sense. Let me try...
< timothy>
now I'm rebuilding rc version using the same "official" machine I use to build official archlinux packages
< timothy>
no, I confirm the bug. from 0.13.1 to 0.13.2rc1 bitcoin-qt tray icon is broken
< timothy>
I'll make more tries and debugging later
< jonasschnelli>
Okay. Thanks
< bitcoin-git>
[bitcoin] rebroad opened pull request #9414: Skip processing of TXs that we already have. (master...DoNotProcessNewTxsMoreThanOnce) https://github.com/bitcoin/bitcoin/pull/9414
< gmaxwell>
Can someone explain to me why we did this?
< gmaxwell>
- LogPrintf("Shutdown : In progress...\n");
< gmaxwell>
+ LogPrintf("%s: In progress...\n", __func__);
< sipa>
i assume so that debug statements don't go out of date when code is moved/renamed?
< gmaxwell>
If the shutdown handling code is moved into a function twizzle-shutprep .. we still probably want the log to print "Shutdown: In progress..." and not "twizzle-shutprep: In progress..." :)
< gmaxwell>
I'd buy that for debug=1 troubleshooting instrumentation.
< gmaxwell>
then again, many of the whitespace changes it made, made things more in line with the approved style but less in line with my personal preferences; so I suppose I'm biased.
< gmaxwell>
:)
< gmaxwell>
man, the backtraces that cross the signals stuff have two pages of boost goop.
< BlueMatt>
yes, it desperately needs replacing
< BlueMatt>
preferably with magic that puts things in threads, but whatever
< BlueMatt>
gmaxwell: hum, might chat with cfields on that one...I know he was looking at replacing that loop maybe even for 0.14
< BlueMatt>
i suppose that change is pretty obvious, though
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #9416: travis: make distdir before make (master...Mf1612-travisDistDirCheck) https://github.com/bitcoin/bitcoin/pull/9416
< gmaxwell>
BlueMatt: yea, I dunno, I wrote it, then threw it out, then decided it might be worth submitting. It still doesn't skip the sleep when the cs_main check in the sideside fails.
< BlueMatt>
that loop sleeps too much for like 3 separate reasons
< BlueMatt>
it needs replacing
< BlueMatt>
s/like 3/literally 3/
< gmaxwell>
well it doesn't really sleep, it gets interrupted by message reception. and basically any send we'd care about actually happens in the call tree of the recieve part of that loop.
< gmaxwell>
so I believe the only places where it actually adds latency to processing is if it suffers lock contention.
< gmaxwell>
e.g. rpc grabs cs_main, and causes none of the send processing to be successful, then it ends up sleeping for 100ms before trying again.
< BlueMatt>
no, look again - it will sleep if you receive a message after processing the node for that message but prior to finishing the whole loop
< BlueMatt>
also if there is lock contention
< BlueMatt>
also lock contention can mean it sleeps both after a wakeup prior to doing real work or might mean it will sleep when it needed not to
< gmaxwell>
yes, thats true. (that a recieve while in the middle of prcessing, won't get handled until the next time around).
< BlueMatt>
yes, but not just next time around, also it might sleep prior to making it there
< gmaxwell>
I thought about just introducing an atomic dontsleep bool that varrious things could set, but I expect that to get squaks.
< BlueMatt>
thats nontrivial
< BlueMatt>
I looked at doing that
< BlueMatt>
because you need to be able to say "yes, I processed for node 0, but havent gotten to node 4" and the thread that does the message-pushing needs to read that, and figure out whether that means you need to not sleep or not
< gmaxwell>
I don't think it's bad? I mean just a if dontsleep sleep=false at the bottom of the loop. And set dontsleep on message reception, and from the sendmessage csmain trylock failure.
< BlueMatt>
I guess its maybe not as bad as I thought, but super ugly
< gmaxwell>
I think it's perfectly fine to just immediately make another pass through the nodes.
< BlueMatt>
I had a patch to set dontsleep=false at the top of the loop
< BlueMatt>
but others might squack since it often means an extra loop iteration
< gmaxwell>
who cares?
< BlueMatt>
oh, I think we're saying the same thign?
< gmaxwell>
I think it's fine to make an extra pass over all peers after doing something.
< BlueMatt>
yea, I mean its probably ok, but not a real fix....cory was looking at a real fix
< BlueMatt>
yes, that would be ok
< BlueMatt>
just gross, and in need of a real fix
< BlueMatt>
there is also the issue of wanting to not old cs_vRecv during message processing, which we do now
< BlueMatt>
cfields said he thought he could get that fixed in time for 0.14
< gmaxwell>
I dunno, make a second pass isn't bad.. we're not talking about millions of peers. :)
< gmaxwell>
I ended up looking at this because I was surprised there was no lock inversion related to cs_vRecv and your pushmessages. but apparently not.
< BlueMatt>
nono, totally, but there are better fixes
< BlueMatt>
yea, its surprisingly not, but only because of TRY_LOCK, which also means it does stupid things on contention