1 2017-03-30 02:22:13 0|luke-jr|I wonder if it would be better to prune debug.log with fallocate/FALLOC_FL_PUNCH_HOLE
2 2017-03-30 05:01:23 0|bitcoin-git|[13bitcoin] 15tjps closed pull request #10059: [trivial] Removed one Boost usage and headers in util.cpp (06master...06tjps_boost) 02https://github.com/bitcoin/bitcoin/pull/10059
3 2017-03-30 06:06:29 0|wumpus|achow101: there's been discussion of having a signal to prune the debug log on command, none exists though. SIGHUP does reopen it, so you could rotate it using a stock log rotator
4 2017-03-30 06:07:56 0|wumpus|with default settings the debug log grows only very slowly so this is not an issue for most users
5 2017-03-30 06:08:17 0|wumpus|and developers that enable extra debug options tend to not want to automatically lose information
6 2017-03-30 06:08:24 0|wumpus|e.g. to collect info over a complete sync cycle
7 2017-03-30 06:10:26 0|gmaxwell|the way the prune thing works kinda stinks in any case. when I get log data from users it very frequently has omitted the interesting part, because they restart before asking for help.
8 2017-03-30 06:10:38 0|gmaxwell|and so I can't see why their node has rejected a valid block.
9 2017-03-30 07:19:11 0|jonasschnelli|BlueMatt: I can't follow your comment: https://github.com/bitcoin/bitcoin/pull/9681/files#r108679775 ... can you elaborate?
10 2017-03-30 07:23:16 0|bitcoin-git|[13bitcoin] 15jtimon opened pull request #10119: Util: Remove ArgsManager wrappers: (06master...060.14-args-wrappers) 02https://github.com/bitcoin/bitcoin/pull/10119
11 2017-03-30 07:25:23 0|bitcoin-git|13bitcoin/06master 14159fe88 15John Newbery: Remove SingleNodeConnCB...
12 2017-03-30 07:25:23 0|bitcoin-git|[13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/f34cdcbd806d...8ac804128671
13 2017-03-30 07:25:24 0|bitcoin-git|13bitcoin/06master 148ac8041 15MarcoFalke: Merge #10109: Remove SingleNodeConnCB...
14 2017-03-30 07:25:47 0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #10109: Remove SingleNodeConnCB (06master...06remove_single_node_conn_cb) 02https://github.com/bitcoin/bitcoin/pull/10109
15 2017-03-30 07:51:25 0|wumpus|FALLOC_FL_PUNCH_HOLE wouldn't improve that; though maybe the aggressive name gives some outlet when there's yet another issue with useless debug logs
16 2017-03-30 07:52:12 0|fanquake|heh https://imgur.com/a/Q5Dsq
17 2017-03-30 07:53:10 0|wumpus|fanquake: it's eating blocks instead of syncing them!
18 2017-03-30 07:54:10 0|wumpus|(yes yes '100% progress' is a moving target, so staying in the same place moves you backwards, relatively, you have to run to stay in the same place)
19 2017-03-30 07:54:35 0|wumpus|I guess we should add a max(0, progress)
20 2017-03-30 08:00:07 0|fanquake|wumpus any schedule for 0.14.1 ? I'm going to remove 9464, not important enough to be fixed.
21 2017-03-30 08:01:28 0|wumpus|fanquake: no precise shedule, though agreement seems to be that there should be a 0.14.1 'soon'
22 2017-03-30 08:02:15 0|fanquake|Only two issues tagged now, are there any more sitting around that should be making it in?
23 2017-03-30 08:02:22 0|wumpus|it's still blocked on the out of memory problems, we're going to PR some workarounds for 0.14
24 2017-03-30 08:02:40 0|wumpus|yep I'm working on one and sipa should have one
25 2017-03-30 08:02:47 0|fanquake|ok
26 2017-03-30 08:04:17 0|wumpus|agree about #9464, as a display problem it's fairly low priority and should at least not block 0.14.1
27 2017-03-30 08:04:18 0|gribble|https://github.com/bitcoin/bitcoin/issues/9464 | [GUI] No GUI updates when running with --reindex or --reindex-chainstate ÷ Issue #9464 ÷ bitcoin/bitcoin ÷ GitHub
28 2017-03-30 08:04:32 0|wumpus|but if someone fixes it it and the solution is not too invasive should be backported
29 2017-03-30 08:05:51 0|bitcoin-git|[13bitcoin] 15fanquake closed pull request #10089: Fix shadowing of 'what' as described in #10080. (06master...06fix-what-shadowing) 02https://github.com/bitcoin/bitcoin/pull/10089
30 2017-03-30 08:06:50 0|wumpus|hrm some RPCs such as getmemoryinfo could be available from the point the RPC server is launched, not just after iniitalization finishes
31 2017-03-30 08:07:40 0|wumpus|also 'stop'
32 2017-03-30 08:16:21 0|bitcoin-git|[13bitcoin] 15laanwj opened pull request #10120: util: Work around (virtual) memory exhaustion on 32-bit w/ glibc (06master...062017_03_address_space_exhaustion_workaround) 02https://github.com/bitcoin/bitcoin/pull/10120
33 2017-03-30 09:22:00 0|luke-jr|wumpus: maybe debug.log should only be pruned when the node considers itself synced + some time has passed?
34 2017-03-30 09:22:20 0|luke-jr|(FALLOC_FL_PUNCH_HOLE possible improvement is that it can do it without rewriting the kept data)
35 2017-03-30 09:23:15 0|jonasschnelli|fanquake: does #9464 fixes the negative progress (https://imgur.com/a/Q5Dsq)?
36 2017-03-30 09:23:16 0|gribble|https://github.com/bitcoin/bitcoin/issues/9464 | [GUI] No GUI updates when running with --reindex or --reindex-chainstate ÷ Issue #9464 ÷ bitcoin/bitcoin ÷ GitHub
37 2017-03-30 09:25:13 0|fanquake|jonasschnelli no, 9464 is just an issue I created that cover a few different GUI issues/quirks, I should add the negative progress output there as well.
38 2017-03-30 09:37:27 0|wumpus|btw travis is being flaky on master
39 2017-03-30 09:37:42 0|wumpus|not sure if this is caused by #9294
40 2017-03-30 09:37:46 0|gribble|https://github.com/bitcoin/bitcoin/issues/9294 | Use internal HD chain for change outputs (hd split) by jonasschnelli ÷ Pull Request #9294 ÷ bitcoin/bitcoin ÷ GitHub
41 2017-03-30 09:38:14 0|jonasschnelli|oh... i'll check
42 2017-03-30 09:39:52 0|jonasschnelli|assumevalid.py failed...
43 2017-03-30 09:39:58 0|jonasschnelli|maybe a rebase/merge issue.
44 2017-03-30 09:42:38 0|gribble|https://github.com/bitcoin/bitcoin/issues/10120 | util: Work around (virtual) memory exhaustion on 32-bit w/ glibc by laanwj ÷ Pull Request #10120 ÷ bitcoin/bitcoin ÷ GitHub
45 2017-03-30 09:42:38 0|wumpus|travis is failing on #10120 as well, but on wallet-hd.py
46 2017-03-30 09:42:42 0|wumpus|could also be a test framework related issue
47 2017-03-30 09:43:38 0|fanquake|jonasschnelli what are your thoughts on the OSX Growl notification code? I'm not sure that it's even being developed/supported anymore. They moved to selling it through the mac app store, and last update was in October 2013.
48 2017-03-30 09:44:47 0|wumpus|test_framework.authproxy.JSONRPCException: non-JSON HTTP response with '503 Service Unavailable' from server (-342)
49 2017-03-30 09:45:06 0|wumpus|while waiting for the node to come up... I think you'll get that if no handler is installed
50 2017-03-30 09:45:29 0|wumpus|there is a small window before handlers are registered
51 2017-03-30 09:45:35 0|wumpus|so not completely unexpected
52 2017-03-30 09:46:16 0|fanquake|wumpus did you just restart the build? I was in the middle on reading the log heh
53 2017-03-30 09:46:28 0|wumpus|yes I restarted the build, sorry :)
54 2017-03-30 09:52:46 0|wumpus|may well be that travis is just being slow and brings this issue to the surface
55 2017-03-30 11:42:46 0|jonasschnelli|wumpus: hmmm.. assumevalid.py failed/failes on master (CRON travis): https://travis-ci.org/bitcoin/bitcoin/jobs/216360061#L5396
56 2017-03-30 11:42:50 0|jonasschnelli|Can't reproduce locally
57 2017-03-30 11:42:59 0|jonasschnelli|(I try now on Ubuntu 14.04
58 2017-03-30 11:50:41 0|wumpus|I couldn't reproduce it locally either
59 2017-03-30 12:30:58 0|jnewbery|#10114 should fix assumevalid.py (and make assumptions in test cases more robust in general)
60 2017-03-30 12:30:59 0|gribble|https://github.com/bitcoin/bitcoin/issues/10114 | An error has occurred and has been logged. Please contact this bot's administrator for more information.
61 2017-03-30 12:31:19 0|wumpus|jnewbery: great!
62 2017-03-30 12:31:59 0|jnewbery|Travis is good (too good!) at uncovering sync and timing issues
63 2017-03-30 12:32:09 0|wumpus|yes
64 2017-03-30 13:29:30 0|BlueMatt|jonasschnelli: ie the user might call bumpfee, get an error saying they need to pay more, then call bumpfee again with a higher number, and get another error saying they need to pay /even/ more
65 2017-03-30 13:29:35 0|BlueMatt|which is ok, but kinda annoying
66 2017-03-30 13:29:47 0|BlueMatt|if they're already checked back-to-back, might as well merge them into one error
67 2017-03-30 13:29:51 0|BlueMatt|so that that cant happen
68 2017-03-30 13:53:17 0|bitcoin-git|[13bitcoin] 15shitakee opened pull request #10121: Add missing header file (06master...06patch-1) 02https://github.com/bitcoin/bitcoin/pull/10121
69 2017-03-30 14:32:15 0|compumatrix|Is it really this silent in here?
70 2017-03-30 14:37:00 0|compumatrix|hi magicwund...
71 2017-03-30 14:38:03 0|magicwund|hey
72 2017-03-30 15:32:57 0|bitcoin-git|[13bitcoin] 15jnewbery opened pull request #10123: Allow debug logs to be excluded from specified component (06master...06debugexclude) 02https://github.com/bitcoin/bitcoin/pull/10123
73 2017-03-30 16:02:43 0|bitcoin-git|[13bitcoin] 15jnewbery opened pull request #10124: [test] Suppress test logging spam (06master...06suppress_test_logging_spam) 02https://github.com/bitcoin/bitcoin/pull/10124
74 2017-03-30 16:51:00 0|jonasschnelli|Reminder for the european CEST people. Meeting will be in 2h 10' (summer time now!).
75 2017-03-30 17:20:27 0|kanzure|i thought we had consensus to eliminate time zones?
76 2017-03-30 17:21:08 0|gmaxwell|kanzure: we did, but the rest of the world hasn't yet.
77 2017-03-30 18:05:53 0|bitcoin-git|[13bitcoin] 15TheBlueMatt opened pull request #10125: Exit bitcoind immediately on shutdown instead of 200ms later (06master...062017-03-faster-shutdown) 02https://github.com/bitcoin/bitcoin/pull/10125
78 2017-03-30 18:34:34 0|bitcoin-git|[13bitcoin] 15jnewbery closed pull request #9630: Add logging to GetAncestor if pindex->pprev == NULL (06master...06crashlogging) 02https://github.com/bitcoin/bitcoin/pull/9630
79 2017-03-30 18:40:35 0|sdaftuar|wumpus: i think #9959 is ready for merge
80 2017-03-30 18:40:36 0|gribble|https://github.com/bitcoin/bitcoin/issues/9959 | Mining: Prevent slowdown in CreateNewBlock on large mempools by sdaftuar ÷ Pull Request #9959 ÷ bitcoin/bitcoin ÷ GitHub
81 2017-03-30 18:48:34 0|wumpus|sdaftuar: ok!
82 2017-03-30 18:56:05 0|bitcoin-git|[13bitcoin] 15laanwj pushed 4 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/8ac804128671...cde9b1a864c1
83 2017-03-30 18:56:06 0|bitcoin-git|13bitcoin/06master 1442cd8c8 15Suhas Daftuar: Add benchmarking for CreateNewBlock
84 2017-03-30 18:56:06 0|bitcoin-git|13bitcoin/06master 14eed816a 15Suhas Daftuar: Mining: return early when block is almost full
85 2017-03-30 18:56:07 0|bitcoin-git|13bitcoin/06master 14011124a 15Suhas Daftuar: Update benchmarking with package statistics
86 2017-03-30 18:56:30 0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #9959: Mining: Prevent slowdown in CreateNewBlock on large mempools (06master...062017-03-cnb-optimizations) 02https://github.com/bitcoin/bitcoin/pull/9959
87 2017-03-30 18:57:19 0|sdaftuar|thanks!
88 2017-03-30 18:57:48 0|BlueMatt|wumpus: what restictions are there on what code you can call from signal handlers?
89 2017-03-30 18:58:26 0|sipa|BlueMatt: as far as I know, only modify simple variables
90 2017-03-30 18:58:35 0|wumpus|BlueMatt: almost nothing can be invoked from signal handlers because signals arrive synchronously, so can be called in any context from any thread
91 2017-03-30 18:59:00 0|BlueMatt|mmm
92 2017-03-30 18:59:03 0|wumpus|everything that has the slightest risk of crashing when called reentrantly can't be used
93 2017-03-30 18:59:13 0|wumpus|so yes, in practice most programs restrict to setting variables
94 2017-03-30 18:59:14 0|BlueMatt|hmm, alright
95 2017-03-30 18:59:22 0|wumpus|another used technique is a pipe
96 2017-03-30 19:00:27 0|jonasschnelli|*dong*
97 2017-03-30 19:00:28 0|gmaxwell|sdaftuar: plz backport #9959
98 2017-03-30 19:00:29 0|gribble|https://github.com/bitcoin/bitcoin/issues/9959 | Mining: Prevent slowdown in CreateNewBlock on large mempools by sdaftuar ÷ Pull Request #9959 ÷ bitcoin/bitcoin ÷ GitHub
99 2017-03-30 19:00:33 0|gmaxwell|#bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr btcdrak sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier
100 2017-03-30 19:00:39 0|kanzure|hi.
101 2017-03-30 19:00:45 0|paveljanik|Hi!
102 2017-03-30 19:00:45 0|sdaftuar|gmaxwell: will do (and here!)
103 2017-03-30 19:00:48 0|wumpus|*reading* from a pipe that has one byte in it, or something like that
104 2017-03-30 19:00:59 0|michagogo|Oh, right, these are back to 10 PM now that DST is back
105 2017-03-30 19:01:00 0|jtimon|hi
106 2017-03-30 19:01:01 0|BlueMatt|wumpus: yea, I get it, but man that sucks
107 2017-03-30 19:01:05 0|cfields|hi
108 2017-03-30 19:01:10 0|wumpus|yes, UNIX signals suck
109 2017-03-30 19:01:15 0|wumpus|#meetingstart
110 2017-03-30 19:01:18 0|lightningbot|Meeting started Thu Mar 30 19:01:18 2017 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
111 2017-03-30 19:01:18 0|lightningbot|Useful Commands: #action #agreed #help #info #idea #link #topic.
112 2017-03-30 19:01:18 0|wumpus|#startmeeting
113 2017-03-30 19:01:41 0|instagibbs|hi
114 2017-03-30 19:01:46 0|jeremyrubin|here
115 2017-03-30 19:01:56 0|wumpus|BlueMatt: it shouldn't matter for the AbortNode() case though. Just make the conditional wait 200 milliseconds or so
116 2017-03-30 19:02:03 0|jtimon|topics?
117 2017-03-30 19:02:25 0|wumpus|doesn't matter if signals are slightly delayed
118 2017-03-30 19:02:30 0|BlueMatt|wumpus: the wait is already 200ms
119 2017-03-30 19:02:30 0|wumpus|yes, topics?
120 2017-03-30 19:02:34 0|BlueMatt|talk about abortnode
121 2017-03-30 19:02:36 0|BlueMatt|thats a reasonable topic
122 2017-03-30 19:02:38 0|wumpus|BlueMatt: yes, but make it a wait on a conditional
123 2017-03-30 19:02:44 0|BlueMatt|mmm, fair
124 2017-03-30 19:02:56 0|BlueMatt|yes, for sigterm its reasonable to wait a few 100 ms
125 2017-03-30 19:03:03 0|BlueMatt|for AbortNode it'll wake it immediately
126 2017-03-30 19:03:06 0|wumpus|right.
127 2017-03-30 19:04:15 0|BlueMatt|in other news, so we got 1/6 "Review Priority" PRs merged this week, that's ok, but we can do better :)
128 2017-03-30 19:04:45 0|BlueMatt|oh, nvm, got 2/6
129 2017-03-30 19:04:50 0|wumpus|FYI: https://github.com/bitcoin/bitcoin/projects/8
130 2017-03-30 19:05:26 0|gmaxwell|BlueMatt: 2/6.
131 2017-03-30 19:05:28 0|jtimon|I don't think anyone commented in mine, at least I got review in others
132 2017-03-30 19:05:28 0|wumpus|removed the two that have been merged
133 2017-03-30 19:05:49 0|gmaxwell|I forgot about the project tracker thing, but reviewed some of them anyways.
134 2017-03-30 19:06:33 0|wumpus|doesn't matter, we'll only add pulls that were mentioned to have priority in the meeting to that project tracker, so if you pay attention at the meeting you don't need it :)
135 2017-03-30 19:07:31 0|wumpus|anyhow, everyone go review them
136 2017-03-30 19:08:21 0|wumpus|topic: 0.14.1?
137 2017-03-30 19:08:29 0|bitcoin-git|[13bitcoin] 15sipa opened pull request #10126: Compensate for memory peak at flush time (06master...06peak_compensation) 02https://github.com/bitcoin/bitcoin/pull/10126
138 2017-03-30 19:08:29 0|gmaxwell|Yes, please.
139 2017-03-30 19:08:33 0|jeremyrubin|topic: slow tests?
140 2017-03-30 19:08:37 0|wumpus|#topic 0.14.1
141 2017-03-30 19:09:03 0|achow101|already?
142 2017-03-30 19:09:25 0|gmaxwell|yes. lots of nice fixes to ship. Minor releases are minor.
143 2017-03-30 19:09:39 0|sipa|we have 11 merged PRs marked 0.14.1
144 2017-03-30 19:09:59 0|wumpus|and three open pulls https://github.com/bitcoin/bitcoin/pulls?q=is%3Aopen+is%3Apr+milestone%3A0.14.1
145 2017-03-30 19:10:32 0|wumpus|sipa: only one still has the "needs backport" tag, I think MarcoFalke ported the rest
146 2017-03-30 19:10:44 0|gmaxwell|I think when we get those three in and those and the recent ones backported, we should be goof for a 0.14.1.
147 2017-03-30 19:11:11 0|gmaxwell|good too.
148 2017-03-30 19:11:42 0|wumpus|agreed
149 2017-03-30 19:12:46 0|wumpus|ok, next topic I guess
150 2017-03-30 19:12:48 0|wumpus|#topic slow tests
151 2017-03-30 19:13:11 0|wumpus|I made an overview here: https://github.com/bitcoin/bitcoin/issues/10026 of the slowest unit tests
152 2017-03-30 19:13:15 0|jeremyrubin|A lot of it is my fault it seems. https://github.com/bitcoin/bitcoin/pull/10099 is ready to go
153 2017-03-30 19:13:27 0|wumpus|some of those have already been worked on or even PRs open to make them faster
154 2017-03-30 19:13:37 0|gmaxwell|jnewbery: if you're around, if 10106 doesn't move forward in a few hours, I recommend you create a new pr, cherry picking that one fix, with your new tests and the fix for the other function. (just because the PR wasn't opened by a regular so they may not be responsive or may not know how to use github well enough to pull your changes.)
155 2017-03-30 19:14:12 0|jnewbery|gmaxwell: I'll do that this afternoon
156 2017-03-30 19:14:43 0|wumpus|yes, tend to agree, they may not know how to cherry-pick them. I could cherry-pick them into his branch but meh
157 2017-03-30 19:14:53 0|MarcoFalke_lab|wumpus: Yes, dropping GetRand() gave a 4* speedup.
158 2017-03-30 19:15:02 0|gmaxwell|jnewbery: and thanks for the awesome response.
159 2017-03-30 19:15:09 0|jnewbery|np
160 2017-03-30 19:15:37 0|wumpus|MarcoFalke_lab: that'll at least move it down a bit in the top 20
161 2017-03-30 19:15:42 0|jeremyrubin|The cuckoocache tests require a bit more discussion to decrease the parameters; I can pick something arbitrary
162 2017-03-30 19:16:27 0|jeremyrubin|The checkqueue tests should also speed up a lot with #9938, but I'm preparing some tweaks to those
163 2017-03-30 19:16:28 0|gribble|https://github.com/bitcoin/bitcoin/issues/9938 | Lock-Free CheckQueue by JeremyRubin ÷ Pull Request #9938 ÷ bitcoin/bitcoin ÷ GitHub
164 2017-03-30 19:16:46 0|Chris_Stewart_5|re tests: Has anyone given any thought to #8469 , obviously this pull request sacrafices speed for exhaustiveness
165 2017-03-30 19:16:48 0|gribble|https://github.com/bitcoin/bitcoin/issues/8469 | [POC] Introducing property based testing to Core by Christewart ÷ Pull Request #8469 ÷ bitcoin/bitcoin ÷ GitHub
166 2017-03-30 19:17:02 0|jeremyrubin|If anyone has high-core machines, they should try benching how that works
167 2017-03-30 19:17:03 0|wumpus|jeremyrubin: alternatively we could have an -extended mode for the unit tests, like we have for the functional tests, to do extra-thorough tests that shouldn't run every time
168 2017-03-30 19:17:13 0|MarcoFalke_lab|jeremyrubin: If the parameters matter, you could provide a switch to test against quick_run and an expensive one
169 2017-03-30 19:17:20 0|wumpus|yes ^^
170 2017-03-30 19:17:27 0|gmaxwell|I think we should have an extended test, and make running it part of the release process.
171 2017-03-30 19:17:42 0|jeremyrubin|Agree
172 2017-03-30 19:17:45 0|wumpus|but for the standard 'make check' there should be a guideline of max ~1 second per test case
173 2017-03-30 19:17:59 0|MarcoFalke_lab|Everyone agrees! :)
174 2017-03-30 19:18:07 0|gmaxwell|I don't care if the tests take an hour to run if it's only a mandatory once in release thing.
175 2017-03-30 19:18:09 0|cfields|sgtm
176 2017-03-30 19:18:11 0|jeremyrubin|no, 2 seconds!
177 2017-03-30 19:18:18 0|wumpus|gmaxwell: yes or once per day or so on master
178 2017-03-30 19:18:36 0|jonasschnelli|(which is failing currently)
179 2017-03-30 19:18:49 0|gmaxwell|jonasschnelli: what is failing?
180 2017-03-30 19:18:49 0|wumpus|yes, travis is broken again, but there's a pull to fix that
181 2017-03-30 19:18:52 0|cfields|note to self: gitian should run whatever extended tests it can
182 2017-03-30 19:18:56 0|gmaxwell|oh travis
183 2017-03-30 19:18:59 0|jonasschnelli|gmaxwell: https://travis-ci.org/bitcoin/bitcoin/builds
184 2017-03-30 19:19:13 0|MarcoFalke_lab|Jup, will merge the travis fix tonight when I have access to my keys. (If no one beats me to it)
185 2017-03-30 19:19:31 0|wumpus|does anyone have the # perhaps?
186 2017-03-30 19:19:58 0|sdaftuar|i think #10114?
187 2017-03-30 19:19:59 0|MarcoFalke_lab|Though, we should be cautious with putting too much into the cron jobs. The extended test rely on the cache being up to date
188 2017-03-30 19:20:00 0|gribble|https://github.com/bitcoin/bitcoin/issues/10114 | An error has occurred and has been logged. Please contact this bot's administrator for more information.
189 2017-03-30 19:20:10 0|MarcoFalke_lab|Otherwise they would break the 49 minute limit on traivs
190 2017-03-30 19:20:11 0|wumpus|there's also #10072
191 2017-03-30 19:20:12 0|gribble|https://github.com/bitcoin/bitcoin/issues/10072 | Remove sources of unreliablility in extended functional tests by jnewbery ÷ Pull Request #10072 ÷ bitcoin/bitcoin ÷ GitHub
192 2017-03-30 19:20:40 0|wumpus|but yes 114 was the one I meant
193 2017-03-30 19:20:41 0|gmaxwell|We should probably contemplate having some seperate process against master that does continual gitian builds or something.
194 2017-03-30 19:21:10 0|MarcoFalke_lab|jonasschnelli: Is doing that, gmaxwell
195 2017-03-30 19:21:22 0|gmaxwell|oh good.
196 2017-03-30 19:21:26 0|gmaxwell|ignore me.
197 2017-03-30 19:21:29 0|jonasschnelli|gmaxwell: https://bitcoin.jonasschnelli.ch
198 2017-03-30 19:21:32 0|wumpus|yes, jonasschnelli has a build server with a very nice web UI
199 2017-03-30 19:21:43 0|MarcoFalke_lab|jonasschnelli: Are you running the tests?
200 2017-03-30 19:21:44 0|jonasschnelli|(it's currently by manual trigger)
201 2017-03-30 19:21:50 0|jonasschnelli|no.. just gitian
202 2017-03-30 19:21:55 0|gmaxwell|somehow I missed this. cool.
203 2017-03-30 19:22:08 0|wumpus|travis already runs the tests, this is to get executables for testing
204 2017-03-30 19:23:31 0|jnewbery|travis is only broken now because we've set it to run the extended tests once per day, so we're currently flushing out all the extended tests that have always failed on travis. I think once #10114 and #10072 are merged the daily runs should succeed reliably
205 2017-03-30 19:23:32 0|gribble|https://github.com/bitcoin/bitcoin/issues/10114 | An error has occurred and has been logged. Please contact this bot's administrator for more information.
206 2017-03-30 19:23:33 0|gribble|https://github.com/bitcoin/bitcoin/issues/10072 | Remove sources of unreliablility in extended functional tests by jnewbery ÷ Pull Request #10072 ÷ bitcoin/bitcoin ÷ GitHub
207 2017-03-30 19:24:09 0|jonasschnelli|thanks jnewbery for the info (and the fixes)
208 2017-03-30 19:25:56 0|bitcoin-git|[13bitcoin] 15sdaftuar opened pull request #10127: [0.14 backport] Mining: Prevent slowdown in CreateNewBlock on large mempools (060.14...062017-03-backport-cnb-optimizations) 02https://github.com/bitcoin/bitcoin/pull/10127
209 2017-03-30 19:26:48 0|wumpus|ok, any other topics?
210 2017-03-30 19:27:41 0|bitcoin-git|[13bitcoin] 15laanwj pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/cde9b1a864c1...edc62c959ab3
211 2017-03-30 19:27:42 0|bitcoin-git|13bitcoin/06master 146426716 15John Newbery: Add send_await_disconnect() method to p2p-compactblocks.py...
212 2017-03-30 19:27:43 0|bitcoin-git|13bitcoin/06master 146a18bb9 15John Newbery: [tests] sync_with_ping should assert that ping hasn't timed out...
213 2017-03-30 19:27:43 0|bitcoin-git|13bitcoin/06master 14edc62c9 15Wladimir J. van der Laan: Merge #10114: [tests] sync_with_ping should assert that ping hasn't timed out...
214 2017-03-30 19:27:50 0|gmaxwell|I should make notes for topics...
215 2017-03-30 19:27:57 0|BlueMatt|oh, new prs
216 2017-03-30 19:28:02 0|BlueMatt|for review priority
217 2017-03-30 19:28:12 0|BlueMatt|looks like jonasschnelli already added his
218 2017-03-30 19:28:16 0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10114: [tests] sync_with_ping should assert that ping hasn't timed out (06master...06improve_sync_with_ping) 02https://github.com/bitcoin/bitcoin/pull/10114
219 2017-03-30 19:28:21 0|BlueMatt|anyone else have something to add (aside from 0.14.1-tagged things)
220 2017-03-30 19:28:23 0|wumpus|fine with me, but don't forget the current bunch :p
221 2017-03-30 19:28:32 0|wumpus|yes please review 0.14.1 tagged things
222 2017-03-30 19:28:36 0|jonasschnelli|BlueMatt: Yes. The bumpfee refactor (to get a QT fee bump function)
223 2017-03-30 19:28:36 0|wumpus|although those should be easy
224 2017-03-30 19:28:53 0|sipa|0.14.1 has priority, but i'd like to see #9792 in at some point to further the get-rid-of-openssl thing :)
225 2017-03-30 19:29:01 0|BlueMatt|wumpus: there is a "For backport" column there...
226 2017-03-30 19:29:02 0|gribble|https://github.com/bitcoin/bitcoin/issues/9792 | FastRandomContext improvements and switch to ChaCha20 by sipa ÷ Pull Request #9792 ÷ bitcoin/bitcoin ÷ GitHub
227 2017-03-30 19:29:09 0|wumpus|but I think we should be able to do 0.14.1 tomorrow so to have something to review for the rest of the week :D
228 2017-03-30 19:29:22 0|wumpus|BlueMatt: yes good point
229 2017-03-30 19:29:53 0|BlueMatt|ok, so morcos and sdaftuar get to propose new ones since they dont have ones up, anyone else want to propose ones?
230 2017-03-30 19:29:56 0|gmaxwell|oh someone opened a PR to do something with debug log excludes, and it adds more insane string processing in the debugging critical path. Would anyone mind if I brought back the PR I shelved to make debug categories use an enum?
231 2017-03-30 19:30:17 0|BlueMatt|gmaxwell: wfm, the pr was jnewbery's
232 2017-03-30 19:30:20 0|sipa|gmaxwell: ack enum debug categories
233 2017-03-30 19:30:40 0|wumpus|gmaxwell: not sure
234 2017-03-30 19:30:43 0|jnewbery|#10123
235 2017-03-30 19:30:44 0|gribble|https://github.com/bitcoin/bitcoin/issues/10123 | Allow debug logs to be excluded from specified component by jnewbery ÷ Pull Request #10123 ÷ bitcoin/bitcoin ÷ GitHub
236 2017-03-30 19:30:48 0|gmaxwell|(the PR was just shelved because I opened it right before a release, and it is a conflict-the-universe PR)
237 2017-03-30 19:30:59 0|wumpus|gmaxwell: well no I guess it makes sense
238 2017-03-30 19:31:00 0|BlueMatt|gmaxwell: go for it now, I'd say
239 2017-03-30 19:31:05 0|wumpus|yes, do it
240 2017-03-30 19:31:07 0|sdaftuar|i'd like to get this DisconnectTip improvement wrapped up (#9208) but i need some help on resolving this annoying issue BlueMatt raised
241 2017-03-30 19:31:08 0|cfields|gmaxwell: +1.
242 2017-03-30 19:31:08 0|gribble|https://github.com/bitcoin/bitcoin/issues/9208 | Improve DisconnectTip performance by sdaftuar ÷ Pull Request #9208 ÷ bitcoin/bitcoin ÷ GitHub
243 2017-03-30 19:31:14 0|jonasschnelli|Yes. ACK
244 2017-03-30 19:31:23 0|cfields|along the same lines, I'd like to do something similar for net messages
245 2017-03-30 19:31:24 0|wumpus|then you can also map the enum values to strings and automate the help message generation
246 2017-03-30 19:31:26 0|gmaxwell|#9424
247 2017-03-30 19:31:27 0|gribble|https://github.com/bitcoin/bitcoin/issues/9424 | Change LogAcceptCategory to use uint32_t rather than sets of strings. by gmaxwell ÷ Pull Request #9424 ÷ bitcoin/bitcoin ÷ GitHub
248 2017-03-30 19:31:30 0|BlueMatt|sdaftuar: want to bring it up now/
249 2017-03-30 19:31:38 0|BlueMatt|?
250 2017-03-30 19:31:38 0|jnewbery|gmaxwell: +1. I'll happily review that one
251 2017-03-30 19:31:49 0|jtimon|since #8855 didn't got new review nor re-review I think just leave that (just remind to potential reviewers that #8994 continues it, in case you "don't see the point")
252 2017-03-30 19:31:49 0|wumpus|cfields: yes, should be fairly easy to do, I already changed the syntax for net messages to be enum-like at some point
253 2017-03-30 19:31:51 0|gribble|https://github.com/bitcoin/bitcoin/issues/8855 | Use a proper factory for creating chainparams by jtimon ÷ Pull Request #8855 ÷ bitcoin/bitcoin ÷ GitHub
254 2017-03-30 19:31:51 0|gribble|https://github.com/bitcoin/bitcoin/issues/8994 | Testchains: Introduce custom chain whose constructor... by jtimon ÷ Pull Request #8994 ÷ bitcoin/bitcoin ÷ GitHub
255 2017-03-30 19:32:15 0|wumpus|cfields: went as far as I could without making it an actual enum :)
256 2017-03-30 19:32:17 0|gmaxwell|cfields: I looked at adding a perfect hash for net messages... but didn't know if that would be a way you'd want to go. :)
257 2017-03-30 19:32:29 0|BlueMatt|new for review this week is 9792 and 9208
258 2017-03-30 19:32:35 0|cfields|wumpus: yes, it sure looks like an enum :)
259 2017-03-30 19:32:37 0|sipa|gmaxwell: just make sure all debug category names have different length
260 2017-03-30 19:33:13 0|gmaxwell|sipa: well we don't need to do any runtime lookup of category names at all.. so no need to do anything performant there.
261 2017-03-30 19:33:27 0|wumpus|at least give them consecutive values so a bit field can be used to represent the set
262 2017-03-30 19:33:33 0|wumpus|intead of a per-thread map :-(
263 2017-03-30 19:33:40 0|cfields|gmaxwell: i had considered making it an array<char, 12>. But I really don't care, as long as it's switchable and a compile-time constant
264 2017-03-30 19:33:41 0|gmaxwell|wumpus: that is what PR 9424 does.
265 2017-03-30 19:33:52 0|wumpus|gmaxwell: ok, great
266 2017-03-30 19:33:53 0|gmaxwell|wumpus: it assigns them each to a bit.
267 2017-03-30 19:34:23 0|wumpus|the current code that assigns it a thread-local variable is really strange
268 2017-03-30 19:34:27 0|bitcoin-git|[13bitcoin] 15JeremyRubin opened pull request #10128: Speed Up CuckooCache tests (06master...06cuckoo-tests-faster) 02https://github.com/bitcoin/bitcoin/pull/10128
269 2017-03-30 19:34:31 0|cfields|(and now that I think about it, std::array probably isn't switchable anyway)
270 2017-03-30 19:34:39 0|gmaxwell|yes. horrors from beyond time.
271 2017-03-30 19:34:54 0|sdaftuar|topic suggestion: dealing with abortnode / ConnectTip / DisconnectTip failures
272 2017-03-30 19:35:05 0|gmaxwell|in any case, if people support-- 9424 is hard to rebase because it touches the whole codebase.
273 2017-03-30 19:35:30 0|wumpus|cfields: in some modern languages it's possible to switch on compound types and arrays and strings, but no, not c++XX before XX=50 or so :)
274 2017-03-30 19:35:33 0|gmaxwell|but I think it also makes jnewbery's exclude trivial. (in fact: no runtime impact...)
275 2017-03-30 19:35:50 0|wumpus|gmaxwell: yes, it's how it should be done
276 2017-03-30 19:36:09 0|sipa|in haskell it's pretty much the only way you can inspect a compound type at all :p
277 2017-03-30 19:36:14 0|jnewbery|gmaxwell: agree
278 2017-03-30 19:36:22 0|wumpus|sipa: indeed
279 2017-03-30 19:36:41 0|sipa|ok, sdaftuar's topic?
280 2017-03-30 19:36:46 0|gmaxwell|cfields: my intution for that would be using gperf to map the messages to integers. then switching on them... but perhaps overkill.
281 2017-03-30 19:36:54 0|wumpus|#topic dealing with abortnode / ConnectTip / DisconnectTip failures
282 2017-03-30 19:37:05 0|BlueMatt|context: #9208
283 2017-03-30 19:37:06 0|gribble|https://github.com/bitcoin/bitcoin/issues/9208 | Improve DisconnectTip performance by sdaftuar ÷ Pull Request #9208 ÷ bitcoin/bitcoin ÷ GitHub
284 2017-03-30 19:37:09 0|sipa|sdaftuar: i saw i was pinged in the PR, but haven't read it
285 2017-03-30 19:37:13 0|sdaftuar|so this issue might be hard to grasp without reviewing #9208
286 2017-03-30 19:37:15 0|gribble|https://github.com/bitcoin/bitcoin/issues/9208 | Improve DisconnectTip performance by sdaftuar ÷ Pull Request #9208 ÷ bitcoin/bitcoin ÷ GitHub
287 2017-03-30 19:37:29 0|wumpus|gmaxwell: will that work for unknown messages too?
288 2017-03-30 19:37:40 0|sdaftuar|but basically matt brought up that we have some edge cases in our code if ConnectTip or DisconnectTip return false
289 2017-03-30 19:37:54 0|wumpus|gmaxwell: does a perfect hash handle collisions? I don't remember
290 2017-03-30 19:38:08 0|cfields|wumpus: perfect hash means no collisions :)
291 2017-03-30 19:38:13 0|wumpus|cfields: for known values
292 2017-03-30 19:38:35 0|jeremyrubin|wouldn't that not be backwards compatible?
293 2017-03-30 19:38:36 0|gmaxwell|wumpus: gperf can check, it's an option.
294 2017-03-30 19:38:44 0|wumpus|cfields: but what if 'moo' maps to the same 32-bit value as 'block'
295 2017-03-30 19:38:50 0|jeremyrubin|someone sends you a new type of message that hashes to something else
296 2017-03-30 19:39:05 0|wumpus|scary
297 2017-03-30 19:39:06 0|sipa|sdaftuar: hmm, i'll review the PR
298 2017-03-30 19:39:10 0|sdaftuar|the last issue i'm trying to resolve is if ConnectTip fails (but the block is not invalid), then we should be in an AbortNode() state. what consistency are we aiming for in this situation?
299 2017-03-30 19:39:14 0|cfields|wumpus: ah, true
300 2017-03-30 19:39:16 0|gmaxwell|wumpus: making it never have collisions for unknows is just an extra comparison with the correct value.
301 2017-03-30 19:39:35 0|gmaxwell|wumpus: which IIRC the gperf emitted code will do, if enabled.
302 2017-03-30 19:39:47 0|BlueMatt|lol, ok, so lets pick a topic instead of two
303 2017-03-30 19:39:47 0|gmaxwell|(actually, looks like it's the default)
304 2017-03-30 19:40:16 0|gmaxwell|sdaftuar: we should be able to shut down and come back up with the underlying issue resolved and continue.
305 2017-03-30 19:40:26 0|wumpus|yes, sorry, I was just curous about gperf
306 2017-03-30 19:40:30 0|gmaxwell|sdaftuar: I don't care if we lose a bunch of recent blocks.
307 2017-03-30 19:40:32 0|sipa|sdaftuar: it's fine if we lose progress in that case, but if at all avoidable, the on disk state should not be corrupted
308 2017-03-30 19:40:44 0|BlueMatt|anyway, so I further observed that shutdown upon AbortNode can take up to 200ms (see recent PR), which is somewhat frightening, given that mempool and chainstate may not be consistent which we assume in many places :/
309 2017-03-30 19:40:46 0|sdaftuar|gmaxwell: sipa: what should we do with the mempool?
310 2017-03-30 19:41:03 0|sipa|sdaftuar: who cares?
311 2017-03-30 19:41:08 0|BlueMatt|so we keep running for a while normally and possibly have incorrect assumptions
312 2017-03-30 19:41:12 0|gmaxwell|I don't care.
313 2017-03-30 19:41:16 0|wumpus|just drop it
314 2017-03-30 19:41:29 0|sipa|sdaftuar: at restart we'll try to reimport what is on disk
315 2017-03-30 19:41:36 0|sipa|and re-run all necessary consistency checks
316 2017-03-30 19:41:51 0|sipa|so mempool.dat doesn't need to be consistent with the chainstate
317 2017-03-30 19:41:59 0|BlueMatt|and wallet?
318 2017-03-30 19:42:01 0|sdaftuar|i think the concern matt was raising was if before shutdown, it's possible for eg an RPC call to get incorrect results
319 2017-03-30 19:42:05 0|sdaftuar|or the wallet
320 2017-03-30 19:42:08 0|wumpus|yes, no tight coupling, good design
321 2017-03-30 19:42:19 0|BlueMatt|wumpus: point is we *have* tight coupling
322 2017-03-30 19:42:23 0|wumpus|wallet doesn't need to be in sync either, though it should not be ahead
323 2017-03-30 19:42:24 0|sipa|sdaftuar: ugh
324 2017-03-30 19:42:28 0|BlueMatt|and continue running after realizing something is corrupted
325 2017-03-30 19:42:35 0|gmaxwell|well abortnode should be instant-ish.
326 2017-03-30 19:42:42 0|gmaxwell|BlueMatt: so you're fixing that, what is there to discuss?
327 2017-03-30 19:42:46 0|wumpus|a bit behind (as long as it's not further than the prune distance) is ok, it will catch up in next start
328 2017-03-30 19:42:49 0|BlueMatt|gmaxwell: i mean the new pr is an improvement, but its not a fix
329 2017-03-30 19:43:03 0|BlueMatt|its not like you cant have something pending cs_main when coming out of Disconnect/ConnectTip
330 2017-03-30 19:43:06 0|wumpus|but if the wallet is ahead of the chain it will trigger a rescan IIRC
331 2017-03-30 19:43:11 0|gmaxwell|so perhaps abortnode needs to Abort()?
332 2017-03-30 19:43:18 0|BlueMatt|gmaxwell: thats pretty much the question
333 2017-03-30 19:43:22 0|BlueMatt|thats super shit, though, of course
334 2017-03-30 19:43:24 0|gmaxwell|and not faffabout witht politely shutting off threads.
335 2017-03-30 19:43:24 0|wumpus|I don't think so
336 2017-03-30 19:43:40 0|sdaftuar|i inadvertently introduced an assert failure in one of these situations. maybe that's a feature not a flaw! :)
337 2017-03-30 19:43:44 0|gmaxwell|Why is it shit? we should be durable across a power off, which is worse than aborting.
338 2017-03-30 19:43:45 0|wumpus|the point of abortnode is to be able to show a GUI dialog to the user
339 2017-03-30 19:43:55 0|gmaxwell|ah
340 2017-03-30 19:43:57 0|wumpus|if you abort() the user will never know to check the debug.log
341 2017-03-30 19:44:08 0|BlueMatt|wumpus: well we can show a gui dialog and abort() prior to unlocking cs_main
342 2017-03-30 19:44:09 0|BlueMatt|:P
343 2017-03-30 19:44:15 0|jeremyrubin|have a graceful shutdown falg
344 2017-03-30 19:44:19 0|jeremyrubin|*flag
345 2017-03-30 19:44:24 0|gmaxwell|wumpus: but it's not unlikely that we can't show a gui... if we can't allocate (likely cause).
346 2017-03-30 19:44:28 0|BlueMatt|which would effectively be sufficient
347 2017-03-30 19:44:29 0|wumpus|I always thought that AbortNode was for errors that could tolerate a graceful shutdown
348 2017-03-30 19:44:35 0|wumpus|the really terrible errors we already assert() or abort() on
349 2017-03-30 19:44:36 0|jeremyrubin|and if we're shutting down gracefully, write to a file "was graceful"
350 2017-03-30 19:44:41 0|BlueMatt|gmaxwell: AbortNode() is usually disk corruption
351 2017-03-30 19:44:46 0|jeremyrubin|On next start, show dialogue first thing?
352 2017-03-30 19:44:46 0|wumpus|please don't make it routine to abort() for everything
353 2017-03-30 19:44:54 0|wumpus|only for things that really warrant it
354 2017-03-30 19:45:00 0|wumpus|not crash on every single error
355 2017-03-30 19:45:02 0|BlueMatt|or too little disk space
356 2017-03-30 19:45:02 0|sdaftuar|or too little disk space, not quite teh same as corruption
357 2017-03-30 19:45:03 0|wumpus|that's just a mess
358 2017-03-30 19:45:21 0|gmaxwell|okay, sure.
359 2017-03-30 19:45:24 0|wumpus|for some problems it's fine to try to clean up normally
360 2017-03-30 19:45:31 0|wumpus|but we still need to exit with a message
361 2017-03-30 19:45:34 0|wumpus|that's what abortnode is for
362 2017-03-30 19:45:56 0|wumpus|if BlueMatt can make it work faster that's great, but don't silently kill the program on every error
363 2017-03-30 19:46:18 0|gmaxwell|wumpus: how about every other error?
364 2017-03-30 19:46:26 0|gmaxwell|:P
365 2017-03-30 19:46:34 0|wumpus|gmaxwell: hehe
366 2017-03-30 19:47:00 0|sipa|do #9208 and #9725 interact?
367 2017-03-30 19:47:01 0|gribble|https://github.com/bitcoin/bitcoin/issues/9208 | Improve DisconnectTip performance by sdaftuar ÷ Pull Request #9208 ÷ bitcoin/bitcoin ÷ GitHub
368 2017-03-30 19:47:03 0|gribble|https://github.com/bitcoin/bitcoin/issues/9725 | CValidationInterface Cleanups by TheBlueMatt ÷ Pull Request #9725 ÷ bitcoin/bitcoin ÷ GitHub
369 2017-03-30 19:47:07 0|gmaxwell|Y'all so worried about the dressings on the coffin. "It's already dead!" But sure. Sorry I was only thinking of OOM, it's just a recent subject.
370 2017-03-30 19:47:20 0|BlueMatt|sipa: they dont, afaik, aside from there now being two structs that can be merged
371 2017-03-30 19:47:22 0|jeremyrubin|I think deleting a file on graceful shutdown should work
372 2017-03-30 19:47:42 0|jeremyrubin|And then starting when that file is present shows the dialouge rather than starting
373 2017-03-30 19:47:47 0|BlueMatt|gmaxwell: yea, AbortNode isnt used for thigns like OOM and total death
374 2017-03-30 19:47:49 0|BlueMatt|disk space also does it
375 2017-03-30 19:47:55 0|BlueMatt|but it can also be db corruption
376 2017-03-30 19:48:13 0|jeremyrubin|This means you don't do anything at all on the Abort, but requires the user to restart their node to see the message
377 2017-03-30 19:48:15 0|BlueMatt|so maybe the solution is AbortNode gets renamed to ShutdownSoon() and use make sure disk corruption is something different?
378 2017-03-30 19:48:31 0|wumpus|BlueMatt: yes, makes sense to me
379 2017-03-30 19:48:34 0|wumpus|BlueMatt: or add a flag
380 2017-03-30 19:48:38 0|morcos|I haven't really thought this all the way through, but doesn't it seem that as we move more and more towards things happening in other threads, that you'd rather let those threads finish what they are doing
381 2017-03-30 19:48:45 0|gmaxwell|BlueMatt: you could also harden things up against your current concer by having the rpc stuff always check the shutdown flag right after taking cs_main (whenever it does)?
382 2017-03-30 19:48:47 0|morcos|If you're committing wallet changes for instance
383 2017-03-30 19:48:59 0|gmaxwell|BlueMatt: and if it finds out that its on its way down, just returning at that point.
384 2017-03-30 19:49:07 0|wumpus|in the long run morcos we should move toward looser coupling of things
385 2017-03-30 19:49:09 0|wumpus|I agree
386 2017-03-30 19:49:14 0|BlueMatt|gmaxwell: yea, I'm worried that if we start down that path we have to check it everywhere
387 2017-03-30 19:49:22 0|BlueMatt|eg wallet may make decisions based on some corrupted mempool state
388 2017-03-30 19:49:27 0|morcos|Let's say you just got some new address from the wallet, and the wallet hasn't committed that state yet and then you Abort()
389 2017-03-30 19:49:31 0|BlueMatt|(I havent thought all the way through all the potential issues here, just a potential concern)
390 2017-03-30 19:50:19 0|wumpus|morcos: luckily we have the keypool to handle that specific contingency
391 2017-03-30 19:50:24 0|jeremyrubin|Every thread could have their own unique ungraceful-close file that it should delete (via RAII) on clean exit. Starting with any present would show error. Uncoupled!
392 2017-03-30 19:50:40 0|gmaxwell|morcos: well at least the worst you get there is replay (due to keypool/hdwallets).. though replay can be pretty bad.
393 2017-03-30 19:51:11 0|wumpus|pretty bad but well at least they will never lose live keys
394 2017-03-30 19:51:47 0|BlueMatt|jeremyrubin: I have a feeling we can be more agressive on the super-strange issues, afaiu this stuff is pretty much hit just with out-of-disk everything else is rare enough.....
395 2017-03-30 19:52:36 0|sipa|maybe we should just have some std::atomic<bool> shits_on_fire_yo; which when set prevents RPCs etc
396 2017-03-30 19:52:46 0|wumpus|jeremyrubin: that's not what I mean with uncoupling, what I mean is that if one thread messes up it doesn't need to take the others with it because it can operate more-or-less independently
397 2017-03-30 19:52:59 0|gmaxwell|sipa: well shutdown can be more or less that.
398 2017-03-30 19:53:14 0|sipa|that can even be set from a signal handler
399 2017-03-30 19:53:34 0|BlueMatt|20<BlueMatt>30 so maybe the solution is AbortNode gets renamed to ShutdownSoon() and use make sure disk corruption is something different?
400 2017-03-30 19:53:53 0|jeremyrubin|sipa: what do you do with an in progress RPC?
401 2017-03-30 19:54:04 0|BlueMatt|in ShutdownSoon cases (eg out of disk) we're ok to continue and just shut down, i think
402 2017-03-30 19:54:08 0|sipa|jeremyrubin: we can only do so much
403 2017-03-30 19:54:08 0|wumpus|you can't generally break off an in-progress RPC
404 2017-03-30 19:54:12 0|BlueMatt|in other cases, we can just assert(false)
405 2017-03-30 19:54:13 0|wumpus|although some will pay attention to fShutdown
406 2017-03-30 19:54:15 0|BlueMatt|because they're super rare anyway
407 2017-03-30 19:54:21 0|gmaxwell|but really, what probably should be done is having the rpcs being able to handle being told "No, you can't have access to [whatever chain state you wanted] right now." then these error conditions that could leave them unstable... could set them.
408 2017-03-30 19:54:21 0|jeremyrubin|wumpus: ^ yes, O
409 2017-03-30 19:54:27 0|BlueMatt|and if your disk is corrupted we probably shouldnt be flushing wallet and chainstate as a part of shutdown anyway
410 2017-03-30 19:54:47 0|BlueMatt|gmaxwell: yea, but thats a huge rewrite for cases that should be super rare
411 2017-03-30 19:55:11 0|gmaxwell|Or we take a whole different approach to failure messages, .e.g. wrap bitcoin-qt in another program that when qt exists it can go through the logs and give you a useful error. (though this doesn't answer morcos' wallet flush, but really that should be in another process.)
412 2017-03-30 19:55:27 0|wumpus|BlueMatt: I think there should be a clear separation between (A) I/O issues such as out of disk spae, which just happen regularly (B) rare implementation issues such as internal consistency errors
413 2017-03-30 19:55:28 0|cfields|iirc rpc has a IsRPCShuttingDown() or so, but only a few things (gbt only, maybe?) checks it
414 2017-03-30 19:55:28 0|jeremyrubin|wumpus: * yes, I'm aware that isn't quite what you meant, just making noise about having a graceful shutdown file because I think it's a reasonable way to mark if a node shut down dirty
415 2017-03-30 19:55:45 0|wumpus|(A) normal errors should just give the user an error as AbortNode does now and shut down properly
416 2017-03-30 19:56:02 0|gmaxwell|It's obnoxious that we can't preallocate resources such that we can guarentee that we never take a failure at an inoppturtune time.
417 2017-03-30 19:56:02 0|wumpus|(B) internal state corruption errors could break off the process immediately
418 2017-03-30 19:56:14 0|gmaxwell|having to slather the code in error handling to deal with that is not ideal.
419 2017-03-30 19:56:41 0|jeremyrubin|gmaxwell: you can preallocate lots of things?
420 2017-03-30 19:56:45 0|gmaxwell|wumpus: "I had to abort processing a block" is a weak kind of internal state corruption. It leaves assumed invariants violated.
421 2017-03-30 19:56:57 0|gmaxwell|jeremyrubin: in particular we can't make leveldb's disk usage constant.
422 2017-03-30 19:56:58 0|BlueMatt|wumpus: yes, thats my proposal
423 2017-03-30 19:57:12 0|BlueMatt|wumpus: but, specifically, B would include things like chainstate-on-disk-corruption
424 2017-03-30 19:57:17 0|BlueMatt|which it already partially does, just not completely
425 2017-03-30 19:57:26 0|wumpus|having code to handle normal errors is perfectly normal and all code has that, I agree that paranoid memory/disk corruption errors tend not to be possible to handle in a sane way
426 2017-03-30 19:57:33 0|wumpus|BlueMatt: yes
427 2017-03-30 19:57:48 0|BlueMatt|ok, soooo, acks on:<BlueMatt> <BlueMatt> so maybe the solution is AbortNode gets renamed to ShutdownSoon() and use make sure disk corruption is something different?
428 2017-03-30 19:58:07 0|wumpus|BlueMatt: as I said beforer, yes, that or add a flag/criticality level
429 2017-03-30 19:58:11 0|BlueMatt|s/use make sure/make sure/
430 2017-03-30 19:58:15 0|BlueMatt|sure, that too
431 2017-03-30 19:58:15 0|jeremyrubin|BlueMatt: maybe if you paste it again
432 2017-03-30 19:58:23 0|BlueMatt|jeremyrubin: ok, <BlueMatt> ok, soooo, acks on:<BlueMatt> <BlueMatt> so maybe the solution is AbortNode gets renamed to ShutdownSoon() and use make sure disk corruption is something different?
433 2017-03-30 19:58:39 0|gmaxwell|wumpus: for example, if we run out of space while flushing our view of the blockchain will be corrupt, and information about the blockchain from rpcs will be wrong. In a world where error handling is free, every code path that gets access to the blockchain data would be able to gracefully handle being told it's just not available. But I think that would be an unreasonable and dangerous amount
434 2017-03-30 19:58:45 0|gmaxwell|of complexity. :(
435 2017-03-30 19:58:50 0|BlueMatt|wumpus: I missed that statement, but, yes
436 2017-03-30 19:59:00 0|gmaxwell|BlueMatt: that sounds okay to me. but I don't know that diskfull is really a shutdown soon though we really do need to give it a nice message. :(
437 2017-03-30 19:59:04 0|wumpus|gmaxwell: note that the out-of-disk error happens *before* the disk is entirely full
438 2017-03-30 19:59:05 0|jeremyrubin|BlueMatt: sgtm
439 2017-03-30 19:59:12 0|wumpus|gmaxwell: there's a threshold
440 2017-03-30 19:59:22 0|wumpus|gmaxwell: so that should already be mitigated
441 2017-03-30 19:59:26 0|gmaxwell|wumpus: yes, but it's not atomic. you can't check and then successfully allocate space.
442 2017-03-30 19:59:38 0|wumpus|no, it's not perfect, but it works pretty well
443 2017-03-30 19:59:43 0|wumpus|I've never had corruption on full disk
444 2017-03-30 20:00:33 0|wumpus|also leveldb write failing shouldn't generally be fatal
445 2017-03-30 20:00:39 0|gmaxwell|on my desktop, which runs with debug=1 it almost always gets checked at the start of the flush. It doesn't corrupt things on disk, but as matt points out the rpc would return somewhat incorrect results during that time.
446 2017-03-30 20:00:40 0|wumpus|it just means the last transaction is not committed
447 2017-03-30 20:01:00 0|jtimon|it seems it's time to abort the meeting
448 2017-03-30 20:01:07 0|lightningbot|Log: http://www.erisian.com.au/meetbot/bitcoin-core-dev/2017/bitcoin-core-dev.2017-03-30-19.01.log.html
449 2017-03-30 20:01:07 0|lightningbot|Meeting ended Thu Mar 30 20:01:06 2017 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
450 2017-03-30 20:01:07 0|lightningbot|Minutes: http://www.erisian.com.au/meetbot/bitcoin-core-dev/2017/bitcoin-core-dev.2017-03-30-19.01.html
451 2017-03-30 20:01:07 0|lightningbot|Minutes (text): http://www.erisian.com.au/meetbot/bitcoin-core-dev/2017/bitcoin-core-dev.2017-03-30-19.01.txt
452 2017-03-30 20:01:07 0|wumpus|#endmeeting
453 2017-03-30 20:01:17 0|BlueMatt|wumpus: we need to change that to #abort()
454 2017-03-30 20:01:20 0|gmaxwell|But I wanted to cleanly flush!
455 2017-03-30 20:02:01 0|bitcoin-git|[13bitcoin] 15theuni opened pull request #10129: scheduler: fix sub-second precision with boost < 1.50 (06master...06fix-scheduler-millisecs) 02https://github.com/bitcoin/bitcoin/pull/10129
456 2017-03-30 20:02:26 0|cfields|BlueMatt: ^^ fixes the ramped-up cpu issue
457 2017-03-30 20:03:00 0|wumpus|cfields: 10129 needs backport I suppose?
458 2017-03-30 20:03:01 0|BlueMatt|cfields: ahh, lol, I dont have old boost
459 2017-03-30 20:03:12 0|BlueMatt|wumpus: only if we also backport the wallet-flush-in-cscheduler, probably?
460 2017-03-30 20:03:18 0|BlueMatt|(which i think we should, but....)
461 2017-03-30 20:03:26 0|BlueMatt|questionable given boost issues
462 2017-03-30 20:03:28 0|wumpus|eh no, let's not do that, sorry
463 2017-03-30 20:03:32 0|cfields|wumpus: won't hurt, but doesn't currently matter
464 2017-03-30 20:04:09 0|BlueMatt|wumpus: fair, yea
465 2017-03-30 20:04:12 0|cfields|wumpus: probably best to go ahead and backport, just in case any other stuff is backported in the future that relies on the correct behavior
466 2017-03-30 20:04:26 0|BlueMatt|yea, i mean if it turns out there are complications and its not dead simple, not backporting seems like the best idea
467 2017-03-30 20:04:34 0|gmaxwell|wallet-flush-in-cscheduler removes a thread?
468 2017-03-30 20:04:38 0|wumpus|cfields: I assued that was the point because one of your other pulls changes the times to std::chrono instead
469 2017-03-30 20:04:47 0|wumpus|this one is backportable at least
470 2017-03-30 20:14:00 0|gmaxwell|BlueMatt: okay sounds fine then... not the stuff that we need a critical realtime response on.
471 2017-03-30 20:14:41 0|wumpus|yes
472 2017-03-30 20:15:20 0|BlueMatt|indeed, the function name changed to compaction from flush when it moved into cscheduler
473 2017-03-30 20:15:24 0|BlueMatt|because thats a more accurate description
474 2017-03-30 20:15:36 0|bitcoin-git|[13bitcoin] 15gmaxwell reopened pull request #9424: Change LogAcceptCategory to use uint32_t rather than sets of strings. (06master...06log_category_simplify) 02https://github.com/bitcoin/bitcoin/pull/9424
475 2017-03-30 20:22:43 0|wumpus|I don't think I remember ever seeing that pull before. Then again, so many pulls get openened, it's easy to lose track :/
476 2017-03-30 20:24:08 0|wumpus|oh end of dec 2016, yea, wasn't too active at that time
477 2017-03-30 20:42:14 0|bitcoin-git|[13bitcoin] 15jnewbery opened pull request #10130: bitcoin-tx input verification (06master...06bitcoin_tx_input_sanitiser) 02https://github.com/bitcoin/bitcoin/pull/10130
478 2017-03-30 20:48:03 0|phantomcircuit|wait there's an issue with flush in csceduler?
479 2017-03-30 20:49:46 0|cfields|phantomcircuit: only with old boost
480 2017-03-30 20:51:53 0|phantomcircuit|oh
481 2017-03-30 21:31:05 0|achow101|if a ping message is sent, but a pong is not received, will other messages still be sent to the pinged node until the ping timeout?
482 2017-03-30 21:38:02 0|sipa|yes
483 2017-03-30 21:38:40 0|achow101|interesting, I'm not seeing that on wireshark..
484 2017-03-30 21:39:33 0|achow101|for that matter, I don't see the ping in question being sent, I just see it in the debug.log
485 2017-03-30 23:09:42 0|bitcoin-git|[13bitcoin] 15fanquake closed pull request #10106: bitcoin-tx: Fix missing range check (06master...06bitcointx-addoutaddr) 02https://github.com/bitcoin/bitcoin/pull/10106