Skip to content

Commit d8409e8

Browse files
hardingharding
committed
Merge #591: Meetings: add 2018-08-03
d5c7cb8 Meetings: add 2018-08-03 (David A. Harding) Pull request description: Preview: http://dg0.dtrt.org/en/meetings/2018/08/02/ Tree-SHA512: dc55af92425fa63d7b87fcbc4e0780a89e7aa692549bb2def4215953903c6e72356da1e64fbd1829381f6194b4084853949eb7d5967e1ab5b3ce423d2c4eef60
2 parents a09a74c + d5c7cb8 commit d8409e8

File tree

2 files changed

+207
-0
lines changed

2 files changed

+207
-0
lines changed

‎_includes/references.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
[BtcDrak]: https://github.com/btcdrak
99
[Chris Stewart]: https://github.com/Christewart
1010
[Clark Moody]: https://github.com/clarkmoody
11+
[Clem Taylor]: https://github.com/clemtaylor
1112
[Chun Kuan Lee]: https://github.com/ken2812221
1213
[Cory Fields]: https://github.com/theuni
1314
[David Harding]: https://github.com/harding/
@@ -41,6 +42,7 @@
4142
[Matt Corallo]: https://github.com/TheBlueMatt
4243
[Michael Ford]: https://github.com/fanquake
4344
[Michagogo]: https://github.com/Michagogo
45+
[Midnight Magic]: https://github.com/midnightmagic
4446
[Nicolas Dorier]: https://github.com/NicolasDorier
4547
[Olaoluwa Osuntokun]: https://github.com/Roasbeef
4648
[Patrick Strateman]: https://github.com/pstratem

‎_posts/en/meetings/2018-08-02-meeting.md

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
---
2+
title: IRC meeting summary for 2018-08-02
3+
lang: en
4+
permalink: /en/meetings/2018/08/02/
5+
name: 2018-08-02-meeting
6+
layout: page
7+
type: meetings
8+
version: 1
9+
---
10+
{% include toc.html %}
11+
{% include references.md %}
12+
13+
- View this week's log on [BotBot.me][bbm log] or [MeetBot][mb log]
14+
- [Meeting minutes by MeetBot][mb minutes]
15+
16+
---
17+
18+
Topics discussed during this weekly meeting included how to deal with
19+
conflicting compiler flags for a particular edge case, continued
20+
discussion from last week about fixing a problem with unicode in
21+
filenames for Windows users, and changing how the program (particularly
22+
its database component) opens files on some systems.
23+
24+
Briefly before the start of topics, Wladimir van der Laan linked to the
25+
[pending PRs][0.17 prs], [pending issues][0.17 issues], and planned
26+
[release schedule][0.17 schedule] for Bitcoin Core 0.17. Potential
27+
contributors are encouraged to review these pages and look ways to help
28+
move the release forward.
29+
30+
## CXXFLAGS stuff
31+
32+
**Background:** the scripts used to build Bitcoin Core pass parameters
33+
(flags) to the compiler to tell it what resources Bitcoin Core needs and
34+
which optimizations to use or avoid. Recently, this included adding a
35+
`-mavx2` flag (Mode AVX2) to enable hardware acceleration for SHA256
36+
hashing on supported CPUs. In addition to the flags Bitcoin Core
37+
passes, users can pass additional parameters using the CXXFLAGS
38+
variable, including the flag `-mno-avx2` (Mode No AVX2). If both of
39+
these flags are passed, only the one that appears last is used.
40+
41+
**Discussion ([log][log cxxflags]):** Luke Dashjr requested and
42+
introduced the topic, "Autotools forces user CXXFLAGS after our own, so
43+
when the user builds with `-mno-avx2`, the build simply fails."
44+
45+
Wladimir van der Laan said, "It looks like a really contrived scenario
46+
to me---not worth it polluting the code with all kinds of compiler
47+
specific pragmas, at least."
48+
49+
Cory Fields added, "I assume the issue is some failures to compile
50+
because of a busted compiler, so there's a desire to be able to avoid
51+
them entirely."
52+
53+
**Conclusion:** Dashjr argued that the issue was a must-fix for Bitcoin
54+
Core 0.17, but van der Laan, Marco Falke, and Gregory Maxwell disagreed.
55+
It seemed likely that additional information from the reporting user
56+
will be solicited to see why they need to pass `-mno-avx2`.
57+
58+
## Windows issues with unicode in filenames
59+
60+
**Background:** as discussed [last meeting][], the interface built into
61+
Microsoft Windows prevents Bitcoin Core from easily opening files
62+
containing non-Latin characters under certain circumstances.
63+
64+
**Discussion ([log][log broken windows]):** Sjors Proovost requested and
65+
introduced the topic: "Do we want to fix the Windows unicode stuff given
66+
that there's still two weeks [left for final development for Bitcoin
67+
Core 0.17]? I think the opinion in the ticket was no."
68+
69+
Cory Fields noted that "due to the nature of the issue, I think many of
70+
the people who would be reporting it may not speak English, so the
71+
significance may be a little under represented."
72+
73+
One particular difficulty with the issue is that there appears to be no
74+
way to reproduce it without access to a Windows system configured a
75+
particular way, so developers who don't use Windows (the majority of
76+
active contributors) can't directly work on it and Bitcoin Core's
77+
automated tests can't be used to prevent future regressions even if the
78+
bug is fixed.
79+
80+
81+
**Conclusion:** Marco Falke noted that fixing the issue "would require a
82+
leveldb [upgrade] and major changes." He suggested aiming to resolve the
83+
issue in 0.18 and several meeting participants seemed to agree. He
84+
further suggested that "we can backport it to 0.17.1, if it qualifies as
85+
a bug fix," to which meeting participants clearly agreed.
86+
87+
## LevelDB FD usage on x86_64
88+
89+
**Background:** Bitcoin Core uses the key-value store database (DB)
90+
LevelDB to track the set of Unspent Transaction Outputs (UTXOs)---all
91+
the spendable groups of bitcoins---as well as for an optional
92+
transaction index Bitcoin Core supports. LevelDB uses lots of
93+
relatively small files (~2 MB) to store its data. When it reads from
94+
those files, it prefers reading in a particular efficient way using
95+
Memory Mapping (`mmap`), but if that's not possible, it falls back to
96+
directly reading from the disk drive using the `select` system call
97+
(syscall) with File Descriptors (FDs). However, the `select` syscall is
98+
severly limited in the number of FDs it can have open.
99+
100+
**Discussion ([log][log ldb]):** Gregory Maxwell requested and introduced the
101+
topic: "There was a recent report from a user hitting the `select` limit
102+
on his x86_64 linux host. Inspection with `lsof` [list open files]
103+
shows that leveldb is using a lot of FDs on nodes where we expected it
104+
to be mostly using `mmap`. Apparently leveldb has a number of mmaps
105+
limit. As far as I know there isn't any reason we shouldn't increase it.
106+
Separately, we should move to using `poll`, but increasing the `mmap` limit
107+
should be a ~1 line change unless someone knows a reason to not do so."
108+
109+
The `mmap` limit was discussed and none of the participants knew of a
110+
reason not to increase it. Just after the end of the meeting, Suhas
111+
Daftuar found [an issue][ldb 128] for leveldb that may indicate why the initial
112+
limit was placed there, which seemed to support increasing the limit on
113+
x86_64 systems.
114+
115+
Interwoven into the discussion of leveldb was the topic of switching
116+
Bitcoin Core from the older `select` syscall to the newer `poll`
117+
syscall for opening File Descriptors (FDs), which includes not just data
118+
files but also network ports. This low-level change would eliminate a
119+
problem where `select` can only handle a low maximum number of FDs and
120+
so Bitcoin Core is limited in various ways (for example, even if you
121+
increase the default maximum number of connections, it can't handle many
122+
more connections). One problem with this change is that Windows does
123+
not implement an equivalent `poll` syscall, so some compatibility
124+
code would need to be written.
125+
126+
**Conclusion:** it seemed likely from the discussion that the LevelDB
127+
`mmap` limit would be increased from its current 1,000 to about 4,000 for
128+
the 0.17 Bitcoin Core release. It seemed unlikely that `select` would
129+
be changed to `poll` for that release as there's not a satisfactory
130+
amount of time for testing, but nobody objected to changing it for a
131+
release in the subsequent planned major version (tentatively 0.18).
132+
133+
*Note:* discussion of this topic continued for about twenty minutes
134+
after the official end of the meeting.
135+
136+
## Humor
137+
138+
Background: the IRC channel has been suffering from spam attacks
139+
recently, so the channel mode was set to quiet (+q) users without
140+
registered accounts.
141+
142+
{% highlight text %}
143+
wumpus topics?
144+
luke-jr crickets
145+
wumpus crickets are... good I guess
146+
gmaxwell can someone please drop the registed users
147+
+q for now? sdaftuar is muted.
148+
provoostenator (I guess it was crickets and the muffled voice
149+
of sdaftuar in the distance)
150+
{% endhighlight %}
151+
152+
## Participants
153+
154+
| IRC nick | Name/Nym |
155+
|-----------------|---------------------------|
156+
| wumpus | [Wladimir van der Laan][] |
157+
| gmaxwell | [Gregory Maxwell][] |
158+
| cfields | [Cory Fields][] |
159+
| luke-jr | [Luke Dashjr][] |
160+
| provoostenator | [Sjors Provoost][] |
161+
| MarcoFalke | [Marco Falke][] |
162+
| meshcollider | [Samuel Dobson][] |
163+
| ken2812221 | [Chun Kuan Lee][] |
164+
| sipa | [Pieter Wuille][] |
165+
| jonasschnelli | [Jonas Schnelli][] |
166+
| ossifrage | [Clem Taylor][] |
167+
| sdaftuar | [Suhas Daftuar][] |
168+
| instagibbs | [Gregory Sanders][] |
169+
| jnewbery | [John Newbery][] |
170+
| phantomcircuit | [Patrick Strateman][] |
171+
| kanzure | [Bryan Bishop][] |
172+
| midnightmagic | [Midnight Magic][] |
173+
| promag | [Joao Barbosa][] |
174+
| achow101 | [Andrew Chow][] |
175+
176+
## Disclaimer
177+
178+
This summary was compiled without input from any of the participants in
179+
the discussion, so any errors are the fault of the summary author and
180+
not the discussion participants. In particular, quotes taken from the
181+
discussion had their capitalization, punctuation, and spelling modified
182+
to produce consistent sentences. Bracketed words and fragments, as well
183+
as background narratives and exposition, were added by the author of
184+
this summary and may have accidentally changed the meaning of some
185+
sentences. If you believe any quote was taken out of context, please
186+
[open an issue][] and we will correct the mistake.
187+
188+
[current high-priority PRs]: https://github.com/bitcoin/bitcoin/projects/8
189+
[open an issue]: https://github.com/bitcoin-core/bitcoincore.org/issues/new
190+
191+
[bbm log]: https://botbot.me/freenode/bitcoin-core-dev/msg/102785470/
192+
[mb minutes]: http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-08-02-19.01.html
193+
{% assign log = "http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-08-02-19.01.log.html" %}
194+
[mb log]: {{log}}
195+
[log cxxflags]: {{log}}#l-24
196+
[log broken windows]: {{log}}#l-87
197+
[log ldb]: {{log}}#l-202
198+
199+
[0.17 prs]: https://github.com/bitcoin/bitcoin/pulls?q=is%3Aopen+is%3Apr+milestone%3A0.17.0
200+
[0.17 issues]: https://github.com/bitcoin/bitcoin/issues?q=is%3Aopen+is%3Aissue+milestone%3A0.17.0
201+
[0.17 schedule]: https://github.com/bitcoin/bitcoin/issues/12624
202+
[last meeting]: /en/meetings/2018/07/26/
203+
[ldb 128]: https://github.com/google/leveldb/issues/128
204+
205+
{% include link-to-issues.md issues="" %}

0 commit comments

Comments
 (0)