Try to increase file descriptor rlimit if necessary #2568

1. 
   sipa commented at 10:52 pm on April 25, 2013: member

   As the default can be too low, especially on OSX.
2. 
   gmaxwell commented at 11:12 pm on April 25, 2013: contributor

   We should probably be clamping max connections at FD_SETSIZE-MIN_CORE_FILEDESCRIPTORS otherwise you get fun stuff like select becoming crashy.
3. 
   jgarzik commented at 11:16 pm on April 25, 2013: contributor

   ACK
4. 
   gavinandresen commented at 12:06 pm on April 26, 2013: contributor

   ACK
5. 
   sipa commented at 12:21 pm on April 26, 2013: member

   @gavinandresen Have you tested whether this compiles on OSX?
6. 
   gavinandresen commented at 3:56 pm on April 26, 2013: contributor

   @sipa : yes, compiles fine, unit tests run, bitcoind -testnet runs on OSX
7. 
   gmaxwell commented at 4:03 pm on April 26, 2013: contributor

   @gavinandresen can you try running

   (for i in {1..220000} ; do export bh=./bitcoind getblockhash $i ; for tx in ./bitcoind getblock $bh | grep ' "' | cut -d'"' -f2 ; do ./bitcoind getrawtransaction $tx > /dev/null ; done ; done)

   On a OSX mainnet node with this patch and txindex=1 when you get a chance? It should causes about 500 FDs to be used.

8. 
   sipa commented at 11:48 pm on April 27, 2013: member

   Refactored the code into a RaiseFileDescriptorLimit in util, and added a windows implementation. Also added Gregory’s suggestion to clamp maxconnections, according to the size of FD_SETSIZE.

   Minimal test plan:

   • Start the client with increasingly higher -maxconnections=X values
   • Look for the line “Using at most Y connections (Z file descriptors available)” in debug.log
   • Without specific hard ulimits, requests for up to ~1000 connections should be granted (X=Y), and Z should always be at least 150 higher than Y.

   I’d like to see at least some proof that this works on Windows on OSX. For Windows, #2569 is needed to get more than 63 connections.

9. Try to increase file descriptor rlimit if necessary

   As the default can be too low, especially on OSX.

   ba29a5590b
10. in src/util.cpp: in 6cc7faccc2 outdated

    1167@@ -1167,6 +1168,28 @@ bool TruncateFile(FILE *file, unsigned int length) {
    1168 #endif
    1169 }
    1170 
    1171+
    1172+// this function tries to raise the file descriptor limit to the requested number.
    1173+// It returns the actual file descriptor limit (which may be more or less than nMinFD)
    1174+int RaiseFileDescriptorLimit(int nMinFD) {
    1175+#if defined(WIN32)
    1176+    return 2048;
    

    ---

    ---

    

    Diapolo commented at 9:02 pm on April 28, 2013:

    Is this correct? MSDN says: “Up to 2,048 files can be open simultaneously at the lowio level (that is, opened and accessed by means of the _open, _read, _write, and so forth family of I/O functions). Up to 512 files can be open simultaneously at the stdio level (that is, opened and accessed by means of the fopen, fgetc, fputc, and so forth family of functions). The limit of 512 open files at the stdio level can be increased to a maximum of 2,048 by means of the _setmaxstdio function.”

    Is the 2048 limit on a lowio level or stdio level? If the later we need to call _setmaxstdio(2048) here? http://msdn.microsoft.com/en-us/library/6e3b887c.aspx

    ---

    

    sipa commented at 9:37 pm on April 28, 2013:

    It’s a bit fuzzy, as things don’t match exactly, but file descriptors correspond to the lowio level. However, the win32 leveldb implementation uses kernel calls directly, bypassing both stdio and lowio, so we don’t actually need to count those. On the other hand, for block and undo files, we do use the stdio level…
11. 
    BitcoinPullTester commented at 0:26 am on April 29, 2013: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/ba29a5590bc4479d74454f0b9fdaf007d9d80221 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
12. 
    gavinandresen commented at 2:29 pm on April 29, 2013: contributor

    ACK. @gmaxwell : successfully ran (a modified version of) your stress test on OSX with this patch and -txindex=1.
13. 
    gmaxwell commented at 7:54 pm on April 29, 2013: contributor

    ACK.
14. gavinandresen referenced this in commit 67f93dc513 on Apr 29, 2013
15. gavinandresen merged this on Apr 29, 2013
16. gavinandresen closed this on Apr 29, 2013

    ---

17. sipa deleted the branch on May 3, 2013
18. laudney referenced this in commit bf567ad258 on Mar 19, 2014
19. bumbacoin referenced this in commit b231daf7a2 on Dec 9, 2016
20. DrahtBot locked this on Sep 8, 2021

Contributors
sipa gmaxwell jgarzik gavinandresen Diapolo BitcoinPullTester

Milestone
v0.8.2