12018-08-02T00:09:23  *** lio17 has quit IRC
  22018-08-02T00:09:58  *** lio17 has joined #bitcoin-core-dev
  32018-08-02T00:18:29  *** lnostdal has quit IRC
  42018-08-02T00:38:20  *** lnostdal has joined #bitcoin-core-dev
  52018-08-02T00:41:25  *** masonicboom has joined #bitcoin-core-dev
  62018-08-02T00:47:31  *** Krellan has quit IRC
  72018-08-02T00:48:29  *** Krellan has joined #bitcoin-core-dev
  82018-08-02T01:22:51  *** goatpig has quit IRC
  92018-08-02T01:39:30  *** promag has joined #bitcoin-core-dev
 102018-08-02T01:47:02  *** d9b4bef9 has quit IRC
 112018-08-02T01:47:05  *** Emcy has quit IRC
 122018-08-02T01:48:08  *** d9b4bef9 has joined #bitcoin-core-dev
 132018-08-02T01:49:14  *** Emcy has joined #bitcoin-core-dev
 142018-08-02T02:40:25  <phantomcircuit> gmaxwell, https://daniel.haxx.se/blog/2012/10/10/wsapoll-is-broken/
 152018-08-02T02:40:31  <phantomcircuit> apparently the answer is dont use that
 162018-08-02T02:44:12  <sipa> Great, WSAPoll doesn't report socket failures
 172018-08-02T02:53:45  <luke-jr> right
 182018-08-02T02:56:08  *** promag has quit IRC
 192018-08-02T03:02:10  <gmaxwell> again: we can stay with select on windows. It doesn't have the 1024 FD problem.
 202018-08-02T03:17:25  <ossifrage> The fix for my specific problem was to just modify how many mmaps() leveldb will make
 212018-08-02T03:18:08  <gmaxwell> ossifrage: do you have any idea why the number of mmaps would be limited at all, on 64 bit systems?
 222018-08-02T03:19:04  <ossifrage> The comment is about "performance reasons" for large databases... But 1000 mmaps is in the noise I think
 232018-08-02T03:19:43  <ossifrage> I changed src/leveldb/util/env_posix.cc mmap_limit from 1000 to 4096
 242018-08-02T03:21:25  <gmaxwell> I don't understand what they mean there.. are they thinking in terms of TLB load or something?
 252018-08-02T03:21:26  <ossifrage> mmap() is a great way to generate a very large amount of write pressure, but it seems like most of the leveldb use in bitcoin has a very low change rate
 262018-08-02T03:23:49  <ossifrage> But lots of memory and slow IO will do that just fine without a single mmmap()
 272018-08-02T03:25:35  <gmaxwell> leveldb's writes are very structured, basically it's an append only thing, that periodically rewrites whole files.
 282018-08-02T03:38:09  <sipa> mmap is only used for readonly things
 292018-08-02T03:38:11  <sipa> afaik
 302018-08-02T03:38:25  <sipa> the files are produced in one go, by dumping a sorted table to disk
 312018-08-02T03:41:06  <gmaxwell> I wish the leveldb project were more active, it would be nice if we could ask if there is a reason we shouldn't just kill the limit on 64-bit.
 322018-08-02T03:48:57  *** Emcy has quit IRC
 332018-08-02T04:05:01  <ossifrage> sipa, that is a good idea, especially if the writes are streaming
 342018-08-02T04:26:57  <phantomcircuit> sipa, gmaxwell should be used only for reads
 352018-08-02T04:27:15  <phantomcircuit> in which case increasing the limit to... infinity shouldn't be an issue on 64bit systems
 362018-08-02T04:31:11  <gmaxwell> doesn't replace using poll instead of select.
 372018-08-02T04:31:18  <gmaxwell> phantomcircuit: hows the PR coming? :P
 382018-08-02T04:32:27  <phantomcircuit> well i had one that looked like it worked but then fucking wsapoll is broken
 392018-08-02T04:32:27  <phantomcircuit> soooo
 402018-08-02T04:32:28  <phantomcircuit> try again
 412018-08-02T04:32:40  <sipa> phantomcircuit: don't do wsapoll
 422018-08-02T04:32:45  <sipa> just poll on sane OSes
 432018-08-02T04:32:54  <sipa> keep using select on windows
 442018-08-02T04:34:08  <gmaxwell> the main reason to not use select is the stupid fd value limit, but that doesn't apply for windows.
 452018-08-02T04:34:58  <phantomcircuit> i mean yeah but that's actually a bigger change
 462018-08-02T04:35:39  <gmaxwell> it's just "keep the existing code, add the poll in an ifdef", no
 472018-08-02T04:38:18  *** Randolf has joined #bitcoin-core-dev
 482018-08-02T04:39:55  <phantomcircuit> gmaxwell, it's slightly different, there's no FD_ISSET
 492018-08-02T04:40:46  <phantomcircuit> you iterate through a list of fd, events pairs
 502018-08-02T04:40:46  <phantomcircuit> the select logic iterates over all the nodes and calls FD_ISSET
 512018-08-02T04:40:46  <phantomcircuit> (which iirc is insane cause FD_ISSET iterates over all the fds)
 522018-08-02T04:44:16  <sipa> phantomcircuit: in windows it does
 532018-08-02T04:44:22  <sipa> in linux it's a bitfield test
 542018-08-02T04:45:51  <phantomcircuit> sipa, oh
 552018-08-02T04:46:04  <phantomcircuit> well either way there isn't a trivial way to do that mapping with poll()
 562018-08-02T04:50:14  <sipa> phantomcircuit: which is btw the reason why select is resteicted to fd's below 1024
 572018-08-02T04:50:24  <sipa> fdset id a 128 byte array
 582018-08-02T04:50:26  <sipa> *is
 592018-08-02T04:52:18  <phantomcircuit> yeah makes sense
 602018-08-02T04:54:37  <phantomcircuit> i'll add the iteration needs to be reversed for epoll or kqueue also anyways
 612018-08-02T05:04:08  *** fanquake has joined #bitcoin-core-dev
 622018-08-02T05:06:01  *** a5m0_ has quit IRC
 632018-08-02T05:07:47  <fanquake> cfields Looking forward to the turtles!
 642018-08-02T05:09:14  <cfields> fanquake: heh, I just pushed it so that dongcarl can get his hands dirty. It's still an absolute disaster.
 652018-08-02T05:09:45  *** a5m0 has joined #bitcoin-core-dev
 662018-08-02T05:13:21  <phantomcircuit> cfields, is there a map from fd to CNode ?
 672018-08-02T05:14:03  <cfields> phantomcircuit: don't believe so. IIRC we always just iterate.
 682018-08-02T05:15:11  <phantomcircuit> how does that work with libevent stuff? iirc it's just calling a callback with the fd right?
 692018-08-02T05:16:22  <cfields> phantomcircuit: the libevent stuff hasn't been merged. You mean in my branches?
 702018-08-02T05:16:53  <phantomcircuit> yeah
 712018-08-02T05:16:56  <cfields> anyway, yea, callback with fd and a few other things, and a caller-supplied pointer
 722018-08-02T05:17:09  <phantomcircuit> oh i see the caller supplied pointer
 732018-08-02T05:17:18  <phantomcircuit> right so libevent is basically keeping that map for you
 742018-08-02T05:18:36  <cfields> well everything's done in reverse, so there shouldn't be any need to ever lookup an fd
 752018-08-02T05:18:51  <cfields> so, i suppose :)
 762018-08-02T05:21:38  <phantomcircuit> cfields, well the underlying epoll thing requires you can map fd to cnode
 772018-08-02T05:21:41  <phantomcircuit> just with libevent it's doing it for you implicitly with the callback data
 782018-08-02T05:21:45  <phantomcircuit> for poll() you need the same but it's explicit
 792018-08-02T05:23:03  <cfields> I thought you had to iterate through the fd list anyway with poll similar to select. Am I completely misremembering?
 802018-08-02T05:25:03  <cfields> after it wakes for active fds, I mean.
 812018-08-02T05:27:35  *** Cory has quit IRC
 822018-08-02T05:30:38  <phantomcircuit> cfields, you iterate through the list of fd's you gave it
 832018-08-02T05:30:38  <phantomcircuit> yes
 842018-08-02T05:31:45  <cfields> phantomcircuit: right, so why the need for a map? You've got the pointers to the CNodes that you pulled the fds from, and you need to test them anyway
 852018-08-02T05:33:06  <cfields> or are you just trying to eliminate the overhead of the iteration of nodes that didn't wake?
 862018-08-02T05:33:57  <cfields> (I don't remember if poll gives you anything to help avoid that)
 872018-08-02T05:35:12  <phantomcircuit> cfields, i mean i can make the map right there, but it's awkward
 882018-08-02T05:36:53  <cfields> phantomcircuit: good luck :)
 892018-08-02T05:36:55  <cfields> nnite
 902018-08-02T05:44:40  *** Emcy has joined #bitcoin-core-dev
 912018-08-02T05:57:03  *** murrayn has joined #bitcoin-core-dev
 922018-08-02T06:41:27  *** intcat has quit IRC
 932018-08-02T06:43:24  *** intcat has joined #bitcoin-core-dev
 942018-08-02T06:49:19  *** Pasha has joined #bitcoin-core-dev
 952018-08-02T06:56:03  <wumpus> kallewoof: if there is no non-locale-independent function, you're going to have to implement one yourself
 962018-08-02T06:56:22  <wumpus> kallewoof: usually this is trivial as the ASCII case of the string functions is trivial
 972018-08-02T06:58:00  <kallewoof> wumpus: ok
 982018-08-02T07:01:12  <jonasschnelli> who own drahtbot?
 992018-08-02T07:01:57  <jonasschnelli> *owns
1002018-08-02T07:02:06  <kallewoof> jonasschnelli: MarcoFalke
1012018-08-02T07:02:28  <jonasschnelli> Nice work... I wasn't aware that it does also builds via gitian
1022018-08-02T07:03:58  <wumpus> yes it's a great bot
1032018-08-02T07:07:18  <fanquake> ^ It keeps getting better
1042018-08-02T07:08:16  <fanquake> I wonder if it'll be running/posting the results of some of the benchmarks from perfmonitor
1052018-08-02T07:10:29  <fanquake> https://github.com/chaincodelabs/bitcoin-perfmonitor
1062018-08-02T07:25:23  <fanquake> wumus 13835 & 13824 should be mergable
1072018-08-02T07:25:26  <fanquake> *wumpus
1082018-08-02T07:30:35  *** andytoshi has quit IRC
1092018-08-02T07:36:01  *** BillSmith4lyfe has quit IRC
1102018-08-02T07:38:12  *** phantomcircuit has quit IRC
1112018-08-02T07:39:24  *** phantomcircuit has joined #bitcoin-core-dev
1122018-08-02T07:45:22  *** Arokh has quit IRC
1132018-08-02T07:46:19  *** Arokh has joined #bitcoin-core-dev
1142018-08-02T07:47:59  *** spinza has quit IRC
1152018-08-02T07:55:35  *** fanquake has quit IRC
1162018-08-02T07:58:08  *** spinza has joined #bitcoin-core-dev
1172018-08-02T08:11:30  *** fanquake has joined #bitcoin-core-dev
1182018-08-02T08:19:10  *** timothy has joined #bitcoin-core-dev
1192018-08-02T08:27:49  <fanquake> wumups Also #13844 and 13796
1202018-08-02T08:27:50  <gribble> https://github.com/bitcoin/bitcoin/issues/13844 | doc: correct the help output for -prune by hebasto · Pull Request #13844 · bitcoin/bitcoin · GitHub
1212018-08-02T08:59:57  *** masonicboom has quit IRC
1222018-08-02T09:02:54  *** masonicboom has joined #bitcoin-core-dev
1232018-08-02T09:06:04  <fanquake> sjors I've added the gitian-build label, so I think that should trigger it.
1242018-08-02T09:16:53  *** AaronvanW has joined #bitcoin-core-dev
1252018-08-02T09:18:24  *** JackH has joined #bitcoin-core-dev
1262018-08-02T09:18:37  <provoostenator> fanquake thanks
1272018-08-02T09:50:46  *** fanquake has quit IRC
1282018-08-02T09:52:36  *** vicenteH has quit IRC
1292018-08-02T09:53:10  *** vicenteH has joined #bitcoin-core-dev
1302018-08-02T10:06:47  *** csknk has joined #bitcoin-core-dev
1312018-08-02T10:14:34  *** e4xit has joined #bitcoin-core-dev
1322018-08-02T10:18:16  *** AaronvanW has quit IRC
1332018-08-02T10:18:51  *** AaronvanW has joined #bitcoin-core-dev
1342018-08-02T10:23:32  *** AaronvanW has quit IRC
1352018-08-02T10:58:04  <wumpus> Aug 01, 12:07 - f030410e88f11c5ff1ce6c80b463a1c7f6d39830functional-test-runner@ccl-bench-hdd-1 Average time up 9.5%
1362018-08-02T10:59:02  <wumpus> looking at how to interpret bitcoinperf.com  - does this mean that merging #13697 made the tests 10% slower on gcc, but not clang?!?
1372018-08-02T10:59:05  <gribble> https://github.com/bitcoin/bitcoin/issues/13697 | Support output descriptors in scantxoutset by sipa · Pull Request #13697 · bitcoin/bitcoin · GitHub
1382018-08-02T11:01:29  <wumpus> it's not surprising that it makes running the functional tests fractially slower, as test cases were added, but 10% is a lot
1392018-08-02T11:04:51  *** CodeShark_ has quit IRC
1402018-08-02T11:04:54  *** Varunram has quit IRC
1412018-08-02T11:04:54  *** ibrightly has quit IRC
1422018-08-02T11:04:54  *** trotski2000 has quit IRC
1432018-08-02T11:04:54  *** NicolasDorier_ has quit IRC
1442018-08-02T11:04:54  *** mariorz_ has quit IRC
1452018-08-02T11:04:54  *** wbnns has quit IRC
1462018-08-02T11:05:42  *** trotski2000 has joined #bitcoin-core-dev
1472018-08-02T11:06:57  *** achow101 has quit IRC
1482018-08-02T11:10:50  *** achow101 has joined #bitcoin-core-dev
1492018-08-02T11:26:57  <provoostenator> xpub derivation is expensive
1502018-08-02T11:27:18  <provoostenator> You could mock the actual derivation with lookup tables.
1512018-08-02T11:27:54  <provoostenator> But that's pretty invasive for the functional test suite.
1522018-08-02T11:29:41  *** AaronvanW has joined #bitcoin-core-dev
1532018-08-02T11:29:41  *** Krellan has quit IRC
1542018-08-02T11:30:26  *** Krellan has joined #bitcoin-core-dev
1552018-08-02T11:31:41  <wumpus> yes but what I mean is that the test framework consists of so many tests, this mean that one test takes up ~1/10th of it
1562018-08-02T11:34:37  *** AaronvanW has quit IRC
1572018-08-02T11:35:12  *** Chris_Stewart_5 has joined #bitcoin-core-dev
1582018-08-02T11:39:38  *** farmerwampum has quit IRC
1592018-08-02T11:46:46  *** goatpig has joined #bitcoin-core-dev
1602018-08-02T11:49:52  <provoostenator> That's not surprising because of how many derivations it's doing, trying to scan up to a thousand (?) keys for those xpub/* patterns. Using the range param for scantxoutset might help.
1612018-08-02T11:50:40  *** masonicboom has quit IRC
1622018-08-02T11:54:25  <wumpus> good point, yes true
1632018-08-02T11:54:48  *** ExtraCrispy has joined #bitcoin-core-dev
1642018-08-02T11:55:03  <provoostenator> I left a note for sipa on the PR. The default is 1000 and sevearl tests use 1499, I suggest using 1 and 2.
1652018-08-02T12:20:33  <wumpus> thanks
1662018-08-02T12:29:52  *** promag has joined #bitcoin-core-dev
1672018-08-02T12:43:25  *** promag has quit IRC
1682018-08-02T12:46:57  *** csknk has quit IRC
1692018-08-02T12:47:44  *** csknk has joined #bitcoin-core-dev
1702018-08-02T12:48:48  <provoostenator> fanquake: no rush, but how often does that bot build things? There's only one ticket with a "Needs gitian build" label atm.
1712018-08-02T12:49:12  *** fanquake has joined #bitcoin-core-dev
1722018-08-02T12:49:58  <fanquake> provoostenator: Not 100% sure if it's completely automated or not. MarcoFalke should be able to help out.
1732018-08-02T12:50:06  <wumpus> looks like we have a causality issue here, that message from provoostenator is visible here *just before* fanquake joining
1742018-08-02T12:51:00  * provoostenator hides time machine
1752018-08-02T12:51:11  <wumpus> leaky wormholes again
1762018-08-02T12:51:13  <fanquake> wumpus :o
1772018-08-02T12:52:27  *** promag has joined #bitcoin-core-dev
1782018-08-02T13:00:06  *** promag has quit IRC
1792018-08-02T13:00:47  *** promag has joined #bitcoin-core-dev
1802018-08-02T13:01:07  *** Chris_Stewart_5 has quit IRC
1812018-08-02T13:01:56  *** promag has quit IRC
1822018-08-02T13:08:15  <fanquake> wumpus I've retested 13823 now, thanks for pointing out the quoting issue.
1832018-08-02T13:12:54  *** promag has joined #bitcoin-core-dev
1842018-08-02T13:28:58  *** csknk has quit IRC
1852018-08-02T13:30:23  *** AaronvanW has joined #bitcoin-core-dev
1862018-08-02T13:33:39  *** ken2812221_ has joined #bitcoin-core-dev
1872018-08-02T13:35:05  *** AaronvanW has quit IRC
1882018-08-02T13:36:53  *** Krellan has quit IRC
1892018-08-02T13:37:53  *** Krellan has joined #bitcoin-core-dev
1902018-08-02T13:37:59  <ken2812221_> wumpus: I think #13426 can be postponed to 0.18, I don't think this can be merged in a week. I'll open an issue to track this and seperate to several PR for easier review
1912018-08-02T13:38:01  <gribble> https://github.com/bitcoin/bitcoin/issues/13426 | [bugfix] Fix encoding issue for Windows by ken2812221 · Pull Request #13426 · bitcoin/bitcoin · GitHub
1922018-08-02T13:43:51  *** SopaXorzTaker has joined #bitcoin-core-dev
1932018-08-02T13:44:12  *** ken2812221_ has quit IRC
1942018-08-02T13:44:46  *** vicenteH has quit IRC
1952018-08-02T13:50:40  <MarcoFalke> provoostenator: It builds on a single cpu on gce, heh. So maybe 24hours to build master and the commit for all targets ;)
1962018-08-02T13:50:41  <ken2812221> Also, we'll 6+months to test if there is any side effect after these changes.
1972018-08-02T13:51:14  <MarcoFalke> ken2812221: Yeah, was going to ask about the state of this.
1982018-08-02T13:51:25  <MarcoFalke> Imo we should at least fix the crash
1992018-08-02T13:51:41  <MarcoFalke> the crash on Windows for non-ascii wallet file name
2002018-08-02T13:54:27  <wumpus> fanquake: cool, thanks for retesting
2012018-08-02T13:54:34  <wumpus> ken2812221: ok!
2022018-08-02T13:55:22  <fanquake> MarcoFalke which PR/changeset do you have in mind for that?
2032018-08-02T13:55:42  <MarcoFalke> Idk what is causing the issue on windows
2042018-08-02T13:55:45  <wumpus> ken2812221: changed milestone--thanks, I sort-of expected this, it was kind of a large and reasonably risky change to merge so last minute, better to do it as one of the first things for 0.18 probably
2052018-08-02T13:55:47  *** vicenteH has joined #bitcoin-core-dev
2062018-08-02T13:56:25  <fanquake> Ah right, we're talking about #13754 here?
2072018-08-02T13:56:26  <gribble> https://github.com/bitcoin/bitcoin/issues/13754 | Windows crashes for -wallet=你好 · Issue #13754 · bitcoin/bitcoin · GitHub
2082018-08-02T13:56:32  *** queip has joined #bitcoin-core-dev
2092018-08-02T13:57:19  <MarcoFalke> jup
2102018-08-02T13:57:45  <MarcoFalke> It reports a fatal error and then hits an assertion
2112018-08-02T13:58:01  <ken2812221> MarcoFalke: That happen on -datadir for a really long time.
2122018-08-02T13:58:19  <MarcoFalke> oh
2132018-08-02T13:58:25  <MarcoFalke> Not a regression then
2142018-08-02T13:58:27  <fanquake> Ok. I'm not entirely sure how we can solve that in not very intrusive, right before 0.17.0 kind of way.
2152018-08-02T13:59:42  <fanquake> 13426 currently has changes to bdb and leveldb as well
2162018-08-02T14:00:06  <provoostenator> Afaik it used to throw a clear error and now it just crashes, but it's only a problem if you use character incompatible with your system language.
2172018-08-02T14:00:32  <MarcoFalke> So not really a common issue to hit
2182018-08-02T14:00:40  <MarcoFalke> Still, would be nice to bisect that
2192018-08-02T14:00:50  <MarcoFalke> Will probably do that
2202018-08-02T14:01:03  <provoostenator> Have fun, only takes 24 hours per build, right? :-)
2212018-08-02T14:02:14  <MarcoFalke> heh, I hope I can steal them from jonasschnelli nightly server
2222018-08-02T14:10:31  <fanquake> If someone wants to do some quick review, there is currently 13852 & 13852 for 0.16, both fairly simple backports
2232018-08-02T14:13:32  *** belcher_ has joined #bitcoin-core-dev
2242018-08-02T14:15:43  *** AaronvanW has joined #bitcoin-core-dev
2252018-08-02T14:17:35  <provoostenator> fanquake duplicate number, what's the second one? I'll take a look.
2262018-08-02T14:18:07  <fanquake> provoostenator Sorry, 13796
2272018-08-02T14:20:24  *** owowo has quit IRC
2282018-08-02T14:20:48  *** csknk has joined #bitcoin-core-dev
2292018-08-02T14:27:36  <fanquake> Another trivial one in 13853, not entirely sure how the versions got out of sync.
2302018-08-02T14:27:48  <provoostenator> MarcoFalke: maybe it's useful to have a "needs windows build" tag? That's the only OS where you can't avoid cross-compilation pain.
2312018-08-02T14:29:06  *** jhfrontz has quit IRC
2322018-08-02T14:29:26  <provoostenator> fanquake: you're sure you want to bump QT to 5.9.6 in backports?
2332018-08-02T14:29:33  <wumpus> let's not get too specific with labels
2342018-08-02T14:29:36  *** jhfrontz has joined #bitcoin-core-dev
2352018-08-02T14:29:59  <provoostenator> wumpus: it's just that the only reason I sometimes ask for Gitian builds is that I want to test Windows binaries.
2362018-08-02T14:30:19  <provoostenator> Maybe others have other reasons.
2372018-08-02T14:30:37  <fanquake> provoostenator bump qt in backports?
2382018-08-02T14:30:46  <provoostenator> https://github.com/bitcoin/bitcoin/pull/13853/files#diff-0c8311709d11060c5156268e58f5f209R26
2392018-08-02T14:30:53  <wumpus> provoostenator: I understand, it's just that I think labels are best for keeping track of rough categories, this seems oddly specific :)
2402018-08-02T14:31:06  <provoostenator> Oh wait, that one isn't a backport, nvm.
2412018-08-02T14:32:04  <provoostenator> Maybe a Github bot listening for comments "Windows plz" is better than tags?
2422018-08-02T14:41:59  *** Aaronvan_ has joined #bitcoin-core-dev
2432018-08-02T14:43:20  <wumpus> I... think we should simply make sure enough CPU capacity is available for the build bot and build everything possible
2442018-08-02T14:44:41  *** Victorsueca has quit IRC
2452018-08-02T14:45:05  *** AaronvanW has quit IRC
2462018-08-02T14:45:50  *** Victorsueca has joined #bitcoin-core-dev
2472018-08-02T14:55:47  *** michaelsdunn1 has joined #bitcoin-core-dev
2482018-08-02T14:56:51  <wumpus> if cloud/server costs are a problem I'm sure we can find some solution
2492018-08-02T14:59:07  <fanquake> Can run all the tests, fuzzers, linters, benchmarks, gitian builds, test sync times..
2502018-08-02T15:00:27  <wumpus> yes, including openbsd and freebsd *ducks*
2512018-08-02T15:01:15  <ken2812221> In my opinion, how about uploading travis build binaries to a server?
2522018-08-02T15:02:45  <ken2812221> It would cost less money than own build.
2532018-08-02T15:04:31  <wumpus> the problem, last time we looked at that option, had to do with credentials; as well as well as with potential malware, especially if PRs automatically upload binaries
2542018-08-02T15:04:47  <wumpus> this is manually triggered by maintainers, so less risky
2552018-08-02T15:08:12  *** fanquake has quit IRC
2562018-08-02T15:14:30  <ken2812221> We can print the binary hash at the end and upload them to a close server. If someone want to get the binary, they can ask maintainer to get it. The downside is that we should trust travis-ci.
2572018-08-02T15:18:52  *** Chris_Stewart_5 has joined #bitcoin-core-dev
2582018-08-02T15:26:25  *** instagibbs has joined #bitcoin-core-dev
2592018-08-02T15:32:27  *** masonicboom has joined #bitcoin-core-dev
2602018-08-02T15:34:26  <MarcoFalke> ken2812221: Could do that with https://transfer.sh/
2612018-08-02T15:36:51  *** masonicboom has quit IRC
2622018-08-02T15:44:08  *** Krellan has quit IRC
2632018-08-02T15:45:28  *** Krellan has joined #bitcoin-core-dev
2642018-08-02T15:55:42  *** goatpig has quit IRC
2652018-08-02T16:03:10  *** romanz has joined #bitcoin-core-dev
2662018-08-02T16:03:20  <ken2812221> MarcoFalke: Thanks, I'll try it.
2672018-08-02T16:25:12  *** vicenteH has quit IRC
2682018-08-02T16:25:44  *** vicenteH has joined #bitcoin-core-dev
2692018-08-02T16:46:06  <wumpus> labeled the risc-v PRs 0.18, would be nice to have executables for that at that time
2702018-08-02T16:48:17  *** ken2812221 has quit IRC
2712018-08-02T16:49:02  *** promag has quit IRC
2722018-08-02T16:52:27  *** Randolf has quit IRC
2732018-08-02T17:16:55  *** Krellan has quit IRC
2742018-08-02T17:21:21  *** masonicboom has joined #bitcoin-core-dev
2752018-08-02T17:21:51  *** JackH has quit IRC
2762018-08-02T17:23:02  *** promag has joined #bitcoin-core-dev
2772018-08-02T17:25:27  *** masonicboom has quit IRC
2782018-08-02T17:30:13  <sipa> wumpus: sgtm
2792018-08-02T17:33:53  *** Aaronvan_ is now known as AaronvanW
2802018-08-02T17:46:59  *** romanz has quit IRC
2812018-08-02T17:48:02  *** d9b4bef9 has quit IRC
2822018-08-02T17:49:08  *** d9b4bef9 has joined #bitcoin-core-dev
2832018-08-02T17:49:53  *** ken2812221 has joined #bitcoin-core-dev
2842018-08-02T17:51:11  *** romanz has joined #bitcoin-core-dev
2852018-08-02T18:27:40  *** SopaXorzTaker has quit IRC
2862018-08-02T18:28:04  *** SopaXorzTaker has joined #bitcoin-core-dev
2872018-08-02T18:29:50  <luke-jr> would be nice to have POWER9 executables ASAP; that hardware is already usable ;)
2882018-08-02T18:56:03  *** clarkmoody has joined #bitcoin-core-dev
2892018-08-02T18:58:27  *** booyah has quit IRC
2902018-08-02T18:59:24  *** schmidty has joined #bitcoin-core-dev
2912018-08-02T19:00:42  <wumpus> luke-jr: yes
2922018-08-02T19:00:58  <wumpus> luke-jr: let's add that for 0.18 too then.
2932018-08-02T19:01:07  <wumpus> #startmeeting
2942018-08-02T19:01:07  <lightningbot> Meeting started Thu Aug  2 19:01:07 2018 UTC.  The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
2952018-08-02T19:01:07  <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
2962018-08-02T19:01:12  <jnewbery> hi!
2972018-08-02T19:01:31  <promag> hi
2982018-08-02T19:01:33  <cfields> hi
2992018-08-02T19:01:35  <provoostenator> hi
3002018-08-02T19:01:42  <jonasschnelli> hi
3012018-08-02T19:02:03  <wumpus> #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 jl2012 achow101 meshcollider jnewbery maaku fanquake promag provoostenator
3022018-08-02T19:02:15  <kanzure> hi.
3032018-08-02T19:02:19  <achow101> hi
3042018-08-02T19:02:23  <meshcollider> Hi
3052018-08-02T19:02:34  <instagibbs> ello
3062018-08-02T19:02:40  *** Jbaczuk has joined #bitcoin-core-dev
3072018-08-02T19:03:10  <gmaxwell> Hi.
3082018-08-02T19:03:24  <wumpus> topics?
3092018-08-02T19:04:08  <luke-jr> crickets
3102018-08-02T19:04:13  *** JackH has joined #bitcoin-core-dev
3112018-08-02T19:04:31  <wumpus> crickets are... good I guess
3122018-08-02T19:04:52  <wumpus> 0.17 PRs: https://github.com/bitcoin/bitcoin/pulls?q=is%3Aopen+is%3Apr+milestone%3A0.17.0
3132018-08-02T19:05:22  <wumpus> 0.17 issues: https://github.com/bitcoin/bitcoin/issues?q=is%3Aopen+is%3Aissue+milestone%3A0.17.0
3142018-08-02T19:05:52  <luke-jr> I guess we could discuss the CXXFLAGS stuff
3152018-08-02T19:05:56  <wumpus> 0.17 release schedule: #12624
3162018-08-02T19:05:57  <gribble> https://github.com/bitcoin/bitcoin/issues/12624 | Release schedule for 0.17.0 · Issue #12624 · bitcoin/bitcoin · GitHub
3172018-08-02T19:06:00  <luke-jr> I don't really have a good solution for it
3182018-08-02T19:06:28  <wumpus> #topic CXXFLAGS stuff
3192018-08-02T19:06:35  <gmaxwell> Whats the issue?
3202018-08-02T19:06:46  *** owowo has joined #bitcoin-core-dev
3212018-08-02T19:06:57  *** sdaftuar has joined #bitcoin-core-dev
3222018-08-02T19:06:58  <luke-jr> gmaxwell: autotools forces user CXXFLAGS after our own; so when the user builds with -mno-avx2, the build simply fails
3232018-08-02T19:07:02  <provoostenator> #13789
3242018-08-02T19:07:04  <gribble> https://github.com/bitcoin/bitcoin/issues/13789 | crypto/sha256: Use pragmas to enforce necessary intrinsics for GCC and Clang by luke-jr · Pull Request #13789 · bitcoin/bitcoin · GitHub
3252018-08-02T19:07:19  <cfields> eh?
3262018-08-02T19:07:29  <cfields> it doesn't force it, we do that ourselves
3272018-08-02T19:07:35  <gmaxwell> can someone please drop the registed users +q for now? sdaftuar is muted.
3282018-08-02T19:07:35  <luke-jr> ie, autotools calls the compiler with our -mavx2 *followed by* -mno-avx2
3292018-08-02T19:07:40  <wumpus> well apparaently there's a problem where the build system passes the wrong flags, and luke-jr's PR works around it with pragmas
3302018-08-02T19:07:43  <gmaxwell> or at least voice sdaftuar
3312018-08-02T19:07:43  <wumpus> that seems wrong to me
3322018-08-02T19:07:50  <cfields> the intention is for any user-passed flags to be able to override the ones we add. If that's not the case, it's a bug.
3332018-08-02T19:07:59  <luke-jr> cfields: that's the problem in this case
3342018-08-02T19:08:06  <luke-jr> we can't let the user override -mavx2 here
3352018-08-02T19:08:09  <sdaftuar> hi
3362018-08-02T19:08:12  *** ChanServ sets mode: +o sdaftuar
3372018-08-02T19:08:23  <cfields> luke-jr: you mean we shouldn't, or we currently don't?
3382018-08-02T19:08:43  <luke-jr> cfields: autotools makes it impossible to override user CXXFLAGS, but for these files we must or we fail to compile
3392018-08-02T19:08:45  <gmaxwell> luke-jr: why would the user ever pass -mno-avx2 in the first place?  We shouldn't be using -mavx except on special files that need it to compile.
3402018-08-02T19:08:47  <cfields> oh, I see what you mean
3412018-08-02T19:08:57  <luke-jr> gmaxwell: to avoid AVX2 instructions
3422018-08-02T19:09:02  <cfields> luke-jr: eh?
3432018-08-02T19:09:07  <luke-jr> gmaxwell: it's those special files that fail to compile
3442018-08-02T19:09:16  <sipa> hi!
3452018-08-02T19:09:26  <cfields> luke-jr: fail how? do we just need to check for more intrinsics?
3462018-08-02T19:09:31  <provoostenator> (I guess it was crickets and the muffled voice of sdaftuar  in the dinstance)
3472018-08-02T19:09:39  <gmaxwell> luke-jr: If the user wants to avoid executing avx2 instructions they need do nothing. They don't have to pass any special options.
3482018-08-02T19:09:59  <gmaxwell> cfields: he is saying that if he builds with CXXFLAGS=-mno-avx2 the compile fails.
3492018-08-02T19:10:10  <luke-jr> gmaxwell: AVX2 is enabled by default with some -march options
3502018-08-02T19:10:12  <cfields> gmaxwell: I assume the issue is some failures to compile because of a busted compiler, so there's a desire to be able to avoid them entirely.
3512018-08-02T19:10:21  <luke-jr> https://github.com/bitcoin/bitcoin/issues/13758
3522018-08-02T19:10:38  <gmaxwell> luke-jr: why would anyone -march=<thing with avx2> then -mno-avx2?  that just seems busted.
3532018-08-02T19:10:38  <cfields> ooooooh
3542018-08-02T19:11:02  *** JackH has quit IRC
3552018-08-02T19:11:04  <wumpus> it looks like a really contrives scenario to me
3562018-08-02T19:11:04  <luke-jr> gmaxwell: I'm not sure why laurentb is doing it, but there's no reason they shouldn't be able to
3572018-08-02T19:11:22  <gmaxwell> in any case, why not detect the -mno-avx2 and then don't even compile the file?
3582018-08-02T19:11:24  <wumpus> not worth it polluting the code with all kinds of compiler specific pragmas at least
3592018-08-02T19:11:25  <cfields> ok, there are plenty of ways to solve that. I think we can just discuss on the github issue?
3602018-08-02T19:11:35  <luke-jr> gmaxwell: autotools says we're supposed to allow changing CXXFLAGS after configure
3612018-08-02T19:11:43  <wumpus> agree with cfields , there's no hurry with this
3622018-08-02T19:11:44  <luke-jr> make CXXFLAGS=…
3632018-08-02T19:11:47  <luke-jr> okay
3642018-08-02T19:12:03  <gmaxwell> in any case -march=<hardware that has avx2> -mno-avx2 sounds like a misguided thing that we shouldn't take a lot of complexity to support, unless someone knows otherwise.
3652018-08-02T19:12:09  <cfields> gmaxwell: we turn off all flags except the ones we're testing when doing the autoconf checks so that they don't cause unrelated errors.
3662018-08-02T19:12:10  <gmaxwell> Esp because there is -mtune
3672018-08-02T19:12:27  <luke-jr> cfields: no we don't
3682018-08-02T19:12:38  <MarcoFalke> Unassigned the issues from the 0.17 milestone for now
3692018-08-02T19:12:42  <cfields> luke-jr: ones that we add, we do
3702018-08-02T19:13:02  <luke-jr> MarcoFalke: it's a must-fix for 0.17
3712018-08-02T19:13:22  <wumpus> no, it's not a must-fix for 0.17
3722018-08-02T19:13:26  <gmaxwell> I don't see how this is a must fix.
3732018-08-02T19:13:33  <wumpus> agree fully with MarcoFalke
3742018-08-02T19:13:36  <luke-jr> broken build system..
3752018-08-02T19:13:48  <wumpus> any other topics?
3762018-08-02T19:13:51  <gmaxwell> "User sets weird options which seem to make no sense as we know, and then something that arguably should work but fails" is not really blocker material.
3772018-08-02T19:13:59  <meshcollider> Lol
3782018-08-02T19:14:00  <provoostenator> Windows
3792018-08-02T19:14:03  <provoostenator> (topic)
3802018-08-02T19:14:04  <MarcoFalke> I left the pulls for review in the 0.17 milestone, so if reviewers like them, they can still be merged
3812018-08-02T19:14:21  <luke-jr> provoostenator: what about Windows? :p
3822018-08-02T19:14:21  <provoostenator> As in: do we want to fix the Windows unicode stuff, given that there's still two weeks?
3832018-08-02T19:14:25  <wumpus> right, it's annoying for the specific user, but if you have really specific needs like that you can patch around it
3842018-08-02T19:14:35  <MarcoFalke> Let's do the unicode stuff for 0.18
3852018-08-02T19:14:36  <wumpus> #topic Windows (provoostenator)
3862018-08-02T19:14:37  <provoostenator> I think the opinion in the ticket was no.
3872018-08-02T19:14:51  <MarcoFalke> It would require a leveldb bump and major changes
3882018-08-02T19:14:55  <gmaxwell> wumpus: we should find out what the user is doing, might just be some greater confusion... like they want to benchmark each way or something.
3892018-08-02T19:15:05  <MarcoFalke> Not sure if we can review and test that in such a short time frame
3902018-08-02T19:15:25  <provoostenator> Ok, and we're not getting tons of reports about this either?
3912018-08-02T19:15:31  <jonasschnelli> Is the unicode stuff just about filenames (wallet)?
3922018-08-02T19:15:32  <wumpus> gmaxwell: right!
3932018-08-02T19:15:43  <MarcoFalke> I am looking into restoring the proper warning for -wallet=non-ascii, but that should be all for 0.17
3942018-08-02T19:15:51  <MarcoFalke> jonasschnelli: Also datadir
3952018-08-02T19:15:52  <sipa> only half here, but feel free to ping me
3962018-08-02T19:15:58  <provoostenator> I think it's mostly filename yes, but also labels.
3972018-08-02T19:16:30  <jonasschnelli> Yeah. We should fix that. But this seems to be open since forever and I don't see a pressing need for 0.17
3982018-08-02T19:16:33  <provoostenator> But afaik I know it works if your system locale is set "correctly".
3992018-08-02T19:16:53  <MarcoFalke> jonasschnelli: Indeed, anything that is not a regression should go into 0.18
4002018-08-02T19:17:04  <cfields> provoostenator: due to the nature of the bug, I think many of the people who would be reporting it may not speak english. So the significance may be a little under-represented.
4012018-08-02T19:17:12  <cfields> s/bug/issue/
4022018-08-02T19:17:16  <gmaxwell> I was trying to say what cfields just said.
4032018-08-02T19:17:22  <meshcollider> Good point
4042018-08-02T19:17:29  <jonasschnelli> Yes. Good point.
4052018-08-02T19:17:35  <luke-jr> provoostenator: wait, labels are broken?
4062018-08-02T19:17:37  <gmaxwell> We shouldn't take few reports to mean few issues... but it the change is invasive and not ready, and the issue isn't new...
4072018-08-02T19:17:58  <cfields> gmaxwell: agreed. -1 from me as well. Just wanted to throw that out there.
4082018-08-02T19:17:59  <provoostenator> luke-jr: not sure, try with an english system locale and then adding Chinese labels, I can't remember if that works.
4092018-08-02T19:18:14  <luke-jr> provoostenator: last I checked, we had functional tests for non-English labels :/
4102018-08-02T19:18:17  <provoostenator> But with a Chinese system locale it does work afaik, hence it doesn't seem super urgent.
4112018-08-02T19:18:24  <wumpus> I think it's a serious issue
4122018-08-02T19:18:27  <MarcoFalke> We can backport to 0.17.1, if it qualifies as bug fix
4132018-08-02T19:18:28  <provoostenator> Functional tests that run on linux.
4142018-08-02T19:18:36  <wumpus> but it's risky to do this last minute
4152018-08-02T19:18:42  <wumpus> and it's not a regression
4162018-08-02T19:18:44  <gmaxwell> MarcoFalke: +1
4172018-08-02T19:19:05  <wumpus> MarcoFalke: agree
4182018-08-02T19:19:08  <provoostenator> Maybe just merge it into master after the 0.17 split so it gets testing quickly. Then we could always backport it if there's strong demand?
4192018-08-02T19:19:14  <gmaxwell> it's a bug, maybe one not worth backporting depending on how tidy the fix is.
4202018-08-02T19:19:37  <wumpus> it's unfortunate that this requires such invasive changes for windows
4212018-08-02T19:19:52  <wumpus> specific changes not required for other OSes
4222018-08-02T19:19:59  <ken2812221> This is really hard to fix.
4232018-08-02T19:20:03  <wumpus> which means most of use cannot test it usefully
4242018-08-02T19:20:24  <luke-jr> can it be reproduced in WINE?
4252018-08-02T19:20:43  <provoostenator> luke-jr: I haven't tried, I just use a virtual box with Windows 10
4262018-08-02T19:20:50  <MarcoFalke> I don't think we should fall back to WINE for testing that issue
4272018-08-02T19:21:10  *** Krellan has joined #bitcoin-core-dev
4282018-08-02T19:21:16  <provoostenator> Someone recently offered to run and maintain Windows integration builds
4292018-08-02T19:21:17  <meshcollider> It doesn't sound like WINE would have the same issue tbh
4302018-08-02T19:21:27  <gmaxwell> Health professions recommend against trying to use wine to solve your problems.
4312018-08-02T19:21:49  <cfields> careful we don't spiral to homebrew...
4322018-08-02T19:21:50  <MarcoFalke> This really needs to be tested on native Windows (Not against testing it in wine additionally, though)
4332018-08-02T19:22:06  <wumpus> yes
4342018-08-02T19:22:18  <provoostenator> #12613
4352018-08-02T19:22:19  <gribble> https://github.com/bitcoin/bitcoin/issues/12613 | [CI] Adding MSVC build to CI check with Appveyor · Issue #12613 · bitcoin/bitcoin · GitHub
4362018-08-02T19:22:36  <ken2812221> If there is a way to run functionfal test on Window
4372018-08-02T19:22:43  <wumpus> MSVC is a orthogonal issue, I think, to solving this in the gitian builds
4382018-08-02T19:22:48  <MarcoFalke> I occasionally run them  on appveyro
4392018-08-02T19:22:57  *** AaronvanW has quit IRC
4402018-08-02T19:23:10  <luke-jr> MarcoFalke: it'd be nice to have Travis test it in the meantime
4412018-08-02T19:23:36  *** AaronvanW has joined #bitcoin-core-dev
4422018-08-02T19:23:39  <wumpus> well not orthogonal but I wouldn't be surprised if there are differences mingw versus MSVC in unicoode handling
4432018-08-02T19:23:40  <MarcoFalke> Sure, if you can get this to work on travis, why not
4442018-08-02T19:24:07  <MarcoFalke> ken2812221: I think I also got them to run on my windows vm once
4452018-08-02T19:24:21  <wumpus> gmaxwell: this is one of the cases where you'd recommend stronger liquior instead.
4462018-08-02T19:24:38  <ken2812221> MSVC adds a unicode version of fstream, but mingw does not have that.
4472018-08-02T19:24:52  <meshcollider> MarcoFalke: "once" sounds promising ;)
4482018-08-02T19:25:16  <MarcoFalke> I could try once more and then write down the steps I did, heh
4492018-08-02T19:25:59  <cfields> ken2812221: huh, we could potentially add it and upstream to mingw, then.
4502018-08-02T19:26:52  <wumpus> why does utf-8 need a special fstream
4512018-08-02T19:26:54  <wumpus> I'm confused
4522018-08-02T19:27:06  <wumpus> why does this have to be so messed up
4532018-08-02T19:27:16  <ken2812221> wumpus: just filename
4542018-08-02T19:27:33  <ken2812221> It's fine to read the stream
4552018-08-02T19:27:34  <wumpus> can't we just drop windows support
4562018-08-02T19:27:38  * wumpus ducks
4572018-08-02T19:27:41  <meshcollider> Lol
4582018-08-02T19:28:02  <provoostenator> Maybe run it inside at little Linux VM? Can't be worse than Electron :-)
4592018-08-02T19:28:07  <wumpus> yesss
4602018-08-02T19:28:21  <wumpus> windows 10 already includes ubuntu right
4612018-08-02T19:28:29  <midnightmagic> it's messed up because windows can still run old windows crap and they have to support old api forever, including all the crappy api they built when the company was on the rocks before they discovered scm
4622018-08-02T19:28:37  <wumpus> let's dump the win32 garbage and use that...
4632018-08-02T19:28:41  <gmaxwell> lol. really with the proposed builder stuff, would building a whole linux system really be worse? :P
4642018-08-02T19:28:50  <cfields> wumpus: Doesn't a brand new win10 update add a bunch of compat stuff that would avoid these issues? I don't think it'd be crazy to consider dropping support for anything less than that reasonably soon.
4652018-08-02T19:29:04  <wumpus> cfields: it does! that's true
4662018-08-02T19:29:05  <provoostenator> Not by default, but you can install it. I once did the inception thing: Gitian builder VM inside Ubuntu inside Windows inside Virtual Box on Mac. It crashed.
4672018-08-02T19:29:27  <gmaxwell> cfields: I dont know the details, but my understanding is that a lot of people don't want to run windows 10 due to integrated adware or something?
4682018-08-02T19:29:56  <provoostenator> gmaxwell: the latest update, at least for me, even forced me to allow analytics data
4692018-08-02T19:30:07  <cfields> whoa
4702018-08-02T19:30:17  <meshcollider> Yeah it's much more invasive
4712018-08-02T19:30:27  <provoostenator> They're definately going for the get hacked by random people on the internet or get spied on by us sales pitch.
4722018-08-02T19:30:38  <cfields> gmaxwell: I suppose those people would be used to running outdated versions of things, then :\
4732018-08-02T19:30:58  <provoostenator> (or both)
4742018-08-02T19:31:22  <wumpus> ugh
4752018-08-02T19:31:35  <wumpus> then again, this is not a regression
4762018-08-02T19:31:40  <wumpus> what works on windows, works.
4772018-08-02T19:31:56  <gmaxwell> thats a possibility too, and better than dropping support for windows...
4782018-08-02T19:32:07  <wumpus> the question is whether we should change 10000 lines just to accomodate unicode in it
4792018-08-02T19:32:38  <gmaxwell> It seems really surprising that we'd need to change more than some wrapper functions.
4802018-08-02T19:32:45  <luke-jr> ^
4812018-08-02T19:32:55  <wumpus> yes it seems a design error in the first place if this cannot be wrapped somehow
4822018-08-02T19:33:27  <cfields> agreed. And that's a reasonable thing to fix, imo. IIRC it also means we have trouble adding filesystem sandboxing.
4832018-08-02T19:33:28  <wumpus> I do think ken2812221's PR is fairly ok in that regard
4842018-08-02T19:34:30  <wumpus> yes the reason I did the fs:: stuff is to allow for filesystem sandboxing
4852018-08-02T19:35:13  *** marcinja_ is now known as marcinja
4862018-08-02T19:35:43  <cfields> wumpus: more is required though, right? I assume something along the lines of ken2812221's PR would help?
4872018-08-02T19:36:55  <wumpus> cfields: I had it down to only changes in fs.h and adding a fs.cpp, afaik
4882018-08-02T19:37:08  <cfields> ah, ok. nm then.
4892018-08-02T19:37:38  <wumpus> but maybe more is needed now that the code evolved further...
4902018-08-02T19:38:07  <gmaxwell> I hope we can find a set of changes that a reasonable independant of windows, so that the windows fix is just a couple files changed.
4912018-08-02T19:38:25  <wumpus> I think my main drawback in ken2812221 's PR is that it changes .string to stringu8 or something, which seems unnecessary, all strings should be utf-8
4922018-08-02T19:38:48  <wumpus> if you make sure the wrapper does that you don't need to change it everywhere in the code
4932018-08-02T19:39:42  <ken2812221> wunpus: That's how boost's path work. It need to pass a utf8 converter.
4942018-08-02T19:40:04  <wumpus> ken2812221: yes...
4952018-08-02T19:40:21  <wumpus> but our own wrapper could do that automatically
4962018-08-02T19:40:23  <ken2812221> Actually I have thought that before, but it need to patch boost.
4972018-08-02T19:40:36  <luke-jr> didn't we want to get rid of boost anyway?
4982018-08-02T19:40:39  <wumpus> or wrap boost
4992018-08-02T19:40:40  <luke-jr> maybe this is a good opportunity
5002018-08-02T19:40:47  <wumpus> we'll do so, in time
5012018-08-02T19:41:12  <wumpus> yes
5022018-08-02T19:41:21  <wumpus> any other topics?
5032018-08-02T19:41:58  <gmaxwell> Topic suggestion: Leveldb FD usage on x86_64
5042018-08-02T19:42:59  <wumpus> #topic leveldb FD usage on x86_64 (gmaxwell)
5052018-08-02T19:43:13  <gmaxwell> There was a recent report from a user hitting the select limit on his x86_64 linux host. Inspection with lsof shows that leveldb is using a lot of FDs on nodes where we expected it to be mostly using mmap.  Apparently leveldb has a number of mmaps limit, as far as I know there isn't any reason we shouldn't increase it.
5062018-08-02T19:43:47  <gmaxwell> (seperately we should move to using poll ... but increasing the mmap limit should be a ~1 line change, unless someone knows a reason to not do so)
5072018-08-02T19:44:23  <wumpus> I think this limit was increased recently
5082018-08-02T19:44:38  <wumpus> specifically for 64-bit OSes, as it was deemed to be no problem
5092018-08-02T19:44:51  <wumpus> of course, I'm not surprised it turns out to be it is... :/
5102018-08-02T19:45:27  <wumpus> the reason to increase it was something with performance
5112018-08-02T19:46:03  <wumpus> #12495 maybe?
5122018-08-02T19:46:06  <gribble> https://github.com/bitcoin/bitcoin/issues/12495 | Increase LevelDB max_open_files by eklitzke · Pull Request #12495 · bitcoin/bitcoin · GitHub
5132018-08-02T19:46:21  <wumpus> "utACK ccedbaf - with the comment that I think relying on undocumented behavior of leveldb is risky. "
5142018-08-02T19:46:35  <wumpus> yeah, of course this is going to bite us in the ass
5152018-08-02T19:47:25  <wumpus> so revert?
5162018-08-02T19:47:33  <cfields> upstream has updated a bunch of stuff lately (namely moving to c++11 by default and dropping the need for lots of platform-dependent code). Has this been addressed as well, by any chance?
5172018-08-02T19:48:08  <gmaxwell> wumpus: hm so reading this it sounds to me that there is a seperate limit of 1000 mmaps. And running into that limit is what is causing more FDs to be used than we expected.
5182018-08-02T19:48:23  <gmaxwell> wumpus: so it might be possible to not revert that patch, but instead increase the maximum mmaps.
5192018-08-02T19:48:24  <cfields> (that bump is going to be painful, btw)
5202018-08-02T19:49:21  <wumpus> gmaxwell: I like switching to anything else than select() though!
5212018-08-02T19:49:29  <wumpus> we've been stuck with select with no good reason
5222018-08-02T19:49:32  <gmaxwell> wumpus: yes, I think we should do that too!
5232018-08-02T19:49:39  *** satwo has joined #bitcoin-core-dev
5242018-08-02T19:49:51  <sipa> gmaxwell: that would explain why i saw exactly 999 mmaps
5252018-08-02T19:49:54  <wumpus> my reason used to be, well, we're going to switch to using libevent for P2P any time now!
5262018-08-02T19:49:58  <gmaxwell> BlueMatt: and phantomcircuit: both previously had private patches to change to poll.
5272018-08-02T19:49:58  <wumpus> but that's been years
5282018-08-02T19:50:01  <wumpus> let's just do it
5292018-08-02T19:50:17  <sipa> wumpus: any time now!
5302018-08-02T19:50:19  <wumpus> change to poll() for unix-ish OSes
5312018-08-02T19:50:24  <gmaxwell> sipa: my comments above about the map limit are based on ossifrage (reporter's) work, and some of the comments in that PR above.
5322018-08-02T19:50:30  <cfields> wumpus: sorry :(
5332018-08-02T19:51:05  <gmaxwell> In any case, I think there are two issues:  (1) switch to poll (duh but can we do it for 0.17) and (2) leveldb should probably be allowed more maps unless we know some reason to not do that.
5342018-08-02T19:51:06  <wumpus> cfields: nooOO sorry I didn't mean it as an attack on you, I could have done it, many other people could have done it, but we didn't
5352018-08-02T19:51:24  <gmaxwell> and maybe (3) potentially revert that PR; but given my understanding, I don't think we need to.
5362018-08-02T19:51:49  <gmaxwell> Leveldb being map limited is probably going to end up bad for performance even if we no longer had the FD issues.
5372018-08-02T19:51:49  <luke-jr> man mmap doesn't talk about a specific mmap limit
5382018-08-02T19:52:10  <wumpus> there is no mmap limit afaik
5392018-08-02T19:52:15  <gmaxwell> ^
5402018-08-02T19:52:38  <gmaxwell> Thats where my concern about increasing leveldb's mmap usage comes from-- I dont understand why the limit is there.
5412018-08-02T19:52:41  <cfields> there's definitely something that limits it. Some unrelated sysctl ?
5422018-08-02T19:52:49  <cfields> I know i've seen it documented somewhere.
5432018-08-02T19:52:49  <gmaxwell> Unless they're trying to reduce TLB thrashing or something.
5442018-08-02T19:54:41  <wumpus> cfields: for 0.18 we should definitely update leveldb though
5452018-08-02T19:54:55  <ossifrage> I bumped my mmap file limit up to 4k, currently 1180 ldb files are mapped and it isn't using any fds for ldb files
5462018-08-02T19:54:57  <gmaxwell> https://github.com/bitcoin/bitcoin/pull/12495#issuecomment-367815005 < eklitzke suggests increasing leveldb's mmap limit here. ossifrage also apparently did that and reported it fixed his problems, though his specific problems are likely better fixed by switching to poll.
5472018-08-02T19:55:04  <wumpus> for 0.17 I'd propose to keep it like this, not do anything wild...
5482018-08-02T19:55:13  <sdaftuar> cfields: if i read correctly, the internet says sysctl vm.max_map_count
5492018-08-02T19:55:40  <gmaxwell> I thought it was probably too late to switch 0.17 to poll, but increasing the leveldb map count seems like an easy mitigation that probably improves performance.
5502018-08-02T19:56:01  <gmaxwell> [gmaxwell@bean ~]$ sysctl vm.max_map_count
5512018-08-02T19:56:01  <gmaxwell> vm.max_map_count = 65530
5522018-08-02T19:56:12  <cfields> is there a per-process one?
5532018-08-02T19:56:35  <ossifrage> vm.max_map_count = 65530 (same on my box, but that number seems a bit low to be a global count)
5542018-08-02T19:56:37  <cfields> gmaxwell: I'd be really nervous about that. There are so many specific quirks related to select/poll/epoll/kqueue
5552018-08-02T19:57:06  <ossifrage> It is per process
5562018-08-02T19:57:19  <cfields> (not against the change at all, only hesitant for the 0.17 cycle)
5572018-08-02T19:57:34  <wumpus> vm.max_map_count = 65530
5582018-08-02T19:57:44  <luke-jr> vm.max_map_count = 65530
5592018-08-02T19:57:46  <wumpus> checked on 3 linux boxes, same output
5602018-08-02T19:57:54  <gmaxwell> likewise.
5612018-08-02T19:57:54  <wumpus> I... strongly doubt this is an issue
5622018-08-02T19:58:24  <gmaxwell> So it might be prudent for 0.17 to bump the leveldb max to 4k or something.
5632018-08-02T19:58:32  <wumpus> if you manage to map 65000+ files, well, I"m not surprised you run into trouble
5642018-08-02T19:58:45  <gmaxwell> I still just wish I better understood why they had that 1000 default.
5652018-08-02T19:59:01  <luke-jr> what happens if the limit is hit?
5662018-08-02T19:59:28  <phantomcircuit> gmaxwell, mine only worked on linux and was actually just broken on windows
5672018-08-02T20:00:05  <wumpus> *dong*
5682018-08-02T20:00:16  *** promag has quit IRC
5692018-08-02T20:00:19  <wumpus> #endmeeting
5702018-08-02T20:00:19  <lightningbot> Meeting ended Thu Aug  2 20:00:19 2018 UTC.  Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
5712018-08-02T20:00:19  <lightningbot> Minutes:        http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-08-02-19.01.html
5722018-08-02T20:00:19  <lightningbot> Minutes (text): http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-08-02-19.01.txt
5732018-08-02T20:00:19  <lightningbot> Log:            http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-08-02-19.01.log.html
5742018-08-02T20:00:49  <ossifrage> luke-jr, it falls back to using fds, does it age old read-only ldb files out or does it just grow until it hits the limit?
5752018-08-02T20:01:43  <midnightmagic> gah, that was a meeting?
5762018-08-02T20:01:52  <sipa> haha
5772018-08-02T20:02:03  <sipa> the best kind of meetings don't feel like meetings at all
5782018-08-02T20:02:35  <sdaftuar> gmaxwell: i just saw this https://github.com/google/leveldb/issues/128
5792018-08-02T20:02:52  <midnightmagic> gah I'm so stupid.
5802018-08-02T20:02:55  <sdaftuar> the comment there makes it sound like the 1000 mmap limit is some kind of memory limiter, but i don't know if i'm understanding correctly
5812018-08-02T20:03:28  <gmaxwell> yep, seems like it.
5822018-08-02T20:03:35  <luke-jr> ossifrage: I don't see that in the code. It looks like it just fails completely
5832018-08-02T20:04:01  <luke-jr>           s = IOError(fname, errno);
5842018-08-02T20:04:10  <ossifrage> luke-jr, I was running into the 1000 map limit and then it started using a ton of FDs
5852018-08-02T20:04:21  <luke-jr> ossifrage: I mean hitting the OS limit
5862018-08-02T20:04:33  <luke-jr> ie, set your OS limit to 900
5872018-08-02T20:05:06  <gmaxwell> I wonder if in the issue sdaftuar links the person is hitting the OS limit.
5882018-08-02T20:05:08  <ossifrage> (ah, I thought you where talking about the limiter limit, duh)
5892018-08-02T20:05:22  <gmaxwell> So leveldb added a maximum and set it to some random value.
5902018-08-02T20:05:35  *** vicenteH has quit IRC
5912018-08-02T20:06:53  <gmaxwell> sdaftuar: thanks for finding that, though I wish it were enlightening.  The user reports having enough memory. Google replies 'we fixed your running out of memory with an arbritary limit'...
5922018-08-02T20:08:09  <luke-jr> seems like they should have at least made it so mmap failure falls back to using fds :/
5932018-08-02T20:08:19  <phantomcircuit> gmaxwell, that does seem to be explicitly saying it's to limit the mmap memory usage to 2000MB
5942018-08-02T20:08:54  <luke-jr> so was this a 32-bit problem only? I thought mmaps were only used on 64-bit?
5952018-08-02T20:08:58  *** clarkmoody has quit IRC
5962018-08-02T20:09:28  <gmaxwell> So I think maybe the limit was put in for 32 bit hosts... and then it was replied to someone who was really reporting another problem.
5972018-08-02T20:09:52  <gmaxwell> luke-jr: we don't use mmaps on 32-bit hosts, but leveldb can...
5982018-08-02T20:10:07  <gmaxwell> (not our leveldb)
5992018-08-02T20:10:28  <wumpus> on 64 bit the limit is much higher
6002018-08-02T20:10:34  <gmaxwell> gah, that was unclear lemme try. We don't use mmap on 32bit with leveldb, but that is a result of our configuration, since we already manage to use all the address space ourselves.
6012018-08-02T20:11:03  <gmaxwell> wumpus: the leveldb mmap limit is also 1000 on 64-bit hosts.
6022018-08-02T20:11:55  <cfields> mmap_limit = sizeof(void*) >= 8 ? 1000 : 0
6032018-08-02T20:11:59  <luke-jr> so sounds like we should just set the mmap limit to infinite on 64-bit, and modify the mmap fail code to fallback?
6042018-08-02T20:12:09  <cfields> so it'd disable mmap for x32 (not x86) as well, right?
6052018-08-02T20:12:30  <cfields> (er, x86 too. but also x32 :)
6062018-08-02T20:12:50  <gmaxwell> luke-jr: well not infinte, but something reasonably under 65530.
6072018-08-02T20:13:37  <luke-jr> I guess we don't want to exhaust it for other apps
6082018-08-02T20:14:06  <luke-jr> 60k / number of bitcoinds launched by functional tests? :P
6092018-08-02T20:14:20  <gmaxwell> setting it to just 4096 would put it well above our current usage. ... and by the time we hit that, we'll hopefully be using poll and it'll just be a performance consideration.
6102018-08-02T20:14:48  <gmaxwell> ossifrage reports he's saying about 1180 maps.
6112018-08-02T20:15:13  <ossifrage> gmaxwell, after 1.5 days of uptime
6122018-08-02T20:15:48  <ossifrage> I started to see the socket problem after 20-30 days of uptime
6132018-08-02T20:16:14  <gmaxwell> well how many ldb files are there in total?
6142018-08-02T20:16:40  *** Krellan has quit IRC
6152018-08-02T20:17:27  <ossifrage> from my last lsof, 992 from chainstate, 146 from txindex (currently). I have 1763 txindex ldb files and 1354 chainstate ldb files
6162018-08-02T20:18:26  <gmaxwell> k, so 4096 would allow you to have all your ldb files mapped.
6172018-08-02T20:18:41  <wumpus> leveldb's mmap limit doesn't correspond to any os-level mmap limit
6182018-08-02T20:19:02  <ossifrage> Yeah, is that limit per database or process wide?
6192018-08-02T20:19:12  <ossifrage> (the leveldb limiter limit)
6202018-08-02T20:20:21  <wumpus> per database
6212018-08-02T20:20:54  <gmaxwell> wumpus: yea, I think the history is that leveldb gained it due to 32bit. 2mb * 1000 = 2GB sounds a lot like someone trying to avoid VM exhaustion on 32bit.
6222018-08-02T20:21:20  <gmaxwell> and they were only thinking about one database and ... yadda yadda.
6232018-08-02T20:24:22  *** vicenteH has joined #bitcoin-core-dev
6242018-08-02T20:25:55  *** csknk has quit IRC
6252018-08-02T20:27:53  *** felco has joined #bitcoin-core-dev
6262018-08-02T20:31:26  *** jnewbery has quit IRC
6272018-08-02T20:39:52  *** masonicboom has joined #bitcoin-core-dev
6282018-08-02T20:44:01  *** masonicboom has quit IRC
6292018-08-02T20:50:12  *** Krellan has joined #bitcoin-core-dev
6302018-08-02T20:53:38  *** jnewbery has joined #bitcoin-core-dev
6312018-08-02T20:56:09  *** SopaXorzTaker has quit IRC
6322018-08-02T21:00:20  <phantomcircuit> gmaxwell, that's so... wtf
6332018-08-02T21:08:35  *** jeremyrubin has joined #bitcoin-core-dev
6342018-08-02T21:14:30  *** Chris_Stewart_5 has quit IRC
6352018-08-02T21:27:12  *** jimpo_ has quit IRC
6362018-08-02T21:29:50  *** jimpo has joined #bitcoin-core-dev
6372018-08-02T21:39:02  *** michaelsdunn1 has quit IRC
6382018-08-02T21:47:52  *** timothy has quit IRC
6392018-08-02T22:04:59  *** belcher_ has quit IRC
6402018-08-02T22:24:18  *** masonicboom has joined #bitcoin-core-dev
6412018-08-02T22:28:46  *** masonicboom has quit IRC
6422018-08-02T22:47:33  *** Chris_Stewart_5 has joined #bitcoin-core-dev
6432018-08-02T23:15:26  *** tryphe has quit IRC
6442018-08-02T23:15:49  *** tryphe has joined #bitcoin-core-dev
6452018-08-02T23:16:57  *** contrapumpkin has quit IRC
6462018-08-02T23:17:27  *** vicenteH has quit IRC
6472018-08-02T23:20:40  *** masonicboom has joined #bitcoin-core-dev
6482018-08-02T23:20:56  *** copumpkin has joined #bitcoin-core-dev
6492018-08-02T23:22:44  *** masonicb_ has joined #bitcoin-core-dev
6502018-08-02T23:25:07  *** masonicboom has quit IRC
6512018-08-02T23:26:51  *** masonicb_ has quit IRC
6522018-08-02T23:41:43  *** vicenteH has joined #bitcoin-core-dev