bug-fix macos: give free bytes to F_PREALLOCATE #17887

kallewoof · 2020-01-07T13:27:57Z

The macos manpage for fcntl (for F_PEOFPOSMODE) states:

Allocate from the physical end of file. In this case, fst_length indicates the number of newly allocated bytes desired.

This would result in the rev files being essentially pre-allocating 2x their necessary size (this is the case for block files as well, but these are flushed down to their right sizes every time) as they would pre-allocate pos + length free bytes, rather than allocating length bytes after pos, as expected.

Fixes #17827.

sipa · 2020-01-07T13:31:05Z

So this code has been wrong for 7 years? Why do we only notice the effect now?

fanquake · 2020-01-07T13:32:53Z

So this code has been wrong for 7 years? Why do we only notice the effect now?

It’s likely this issue only started occurring with the introduction of APFS, which is quite recent.

kallewoof · 2020-01-07T13:33:28Z

@sipa I suspect the OS eventually reclaims pre-allocated space, but it seems block files are not suffering from this kind of issue, so I'm honestly not sure. The docs clearly indicate we were using it wrong, though. (Edit: the issue is a bit more convoluted; see #17887 (comment))

Old edit:

It’s likely this issue only started occurring with the introduction of APFS, which is quite recent.

That is probably more likely.

kallewoof · 2020-01-07T13:43:17Z

On my machine (APFS encrypted volume), I was seeing rev00002.dat hitting 20 MB around block 135k. After this patch (running up to block 191106, then shutting down) I see

$ du -ch *.dat
128M	blk00000.dat
128M	blk00001.dat
128M	blk00002.dat
128M	blk00003.dat
128M	blk00004.dat
128M	blk00005.dat
128M	blk00006.dat
128M	blk00007.dat
128M	blk00008.dat
128M	blk00009.dat
128M	blk00010.dat
128M	blk00011.dat
128M	blk00012.dat
128M	blk00013.dat
128M	blk00014.dat
128M	blk00015.dat
128M	blk00016.dat
 48M	blk00017.dat
 20M	rev00000.dat
 17M	rev00001.dat
 17M	rev00002.dat
 17M	rev00003.dat
 17M	rev00004.dat
 18M	rev00005.dat
 18M	rev00006.dat
 19M	rev00007.dat
 19M	rev00008.dat
 18M	rev00009.dat
 18M	rev00010.dat
 19M	rev00011.dat
 19M	rev00012.dat
 19M	rev00013.dat
 19M	rev00014.dat
 18M	rev00015.dat
 19M	rev00016.dat
5.1M	rev00017.dat
2.5G	total

Master (~same block, after shutdown):

$ du -ch *.dat
128M	blk00000.dat
128M	blk00001.dat
128M	blk00002.dat
128M	blk00003.dat
128M	blk00004.dat
128M	blk00005.dat
128M	blk00006.dat
128M	blk00007.dat
128M	blk00008.dat
128M	blk00009.dat
128M	blk00010.dat
128M	blk00011.dat
128M	blk00012.dat
128M	blk00013.dat
128M	blk00014.dat
128M	blk00015.dat
128M	blk00016.dat
144M	blk00017.dat
 19M	rev00000.dat
 17M	rev00001.dat
 16M	rev00002.dat
 17M	rev00003.dat
 17M	rev00004.dat
 18M	rev00005.dat
 18M	rev00006.dat
 36M	rev00007.dat
 51M	rev00008.dat
 18M	rev00009.dat
 18M	rev00010.dat
 18M	rev00011.dat
162M	rev00012.dat
 67M	rev00013.dat
126M	rev00014.dat
 51M	rev00015.dat
 67M	rev00016.dat
 21M	rev00017.dat
3.0G	total

Both runs were after rm -rf $BITCOINDIR.

grep 'Pre-allocating' results for master run, minus a small part of the start: https://pastebin.com/dXLLcRQa

Looking closely at the run on master, the files are blowing up 2x in size until they are no longer used, at which point they are sometimes truncated to their actual size. The blow-up applies to blk files as well, but these are always truncated down after use (see blk00017.dat, which is too large because it isn't finished yet; this was after shutting Bitcoin Core down).

Running on this PR, the 2x blowup is not occurring for either files.

Running macos 10.14.6 (Mojave). User is running Catalina, so perhaps I would see the problem more often if I upgraded.

DrahtBot · 2020-01-07T13:54:10Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

kallewoof · 2020-01-07T16:11:19Z

Enlightening read: https://www.mail-archive.com/[email protected]/msg00226.html

IPGlider · 2020-01-07T19:03:58Z

Tested on MacOS 10.15.2 on an APFS volume and the problem disappears. Also tested on a mounted HFS+ disk volume and the problem disappears.

fanquake · 2020-01-08T02:45:32Z

Concept ACK

eriknylund · 2020-01-09T20:18:38Z

I ran this on machine 2 in the issue report and can confirm that there is no blowup.

MacBook Pro (15-inch, 2018)
2,6 GHz 6-Core Intel Core i7
32 GB 2400 MHz DDR4
macOS Catalina 10.15.2 (upgraded from Mojave)

Tested on an APFS unencrypted disk image with 3 GB in size and let it run until the image was full:

$ ./bitcoind -datadir=/Volumes/APFS
2020-01-09T19:43:17Z UpdateTip: 
new best=000000000000059cfb1620756c7a88b679dfcbedc1d3c255dbc0f09481f8021b
height=191055 version=0x00000001 log2_work=68.451374 tx=5349799 
date='2012-07-27T17:15:53Z' progress=0.010844 cache=292.8MiB(2160691txo)
2020-01-09T19:43:19Z *** Disk space is too low!

$ cd /Volumes/APFS/blocks/

$ du -ch *.dat
128M	blk00000.dat
128M	blk00001.dat
128M	blk00002.dat
128M	blk00003.dat
128M	blk00004.dat
128M	blk00005.dat
128M	blk00006.dat
128M	blk00007.dat
128M	blk00008.dat
128M	blk00009.dat
128M	blk00010.dat
128M	blk00011.dat
128M	blk00012.dat
128M	blk00013.dat
128M	blk00014.dat
128M	blk00015.dat
128M	blk00016.dat
 48M	blk00017.dat
 20M	rev00000.dat
 17M	rev00001.dat
 16M	rev00002.dat
 17M	rev00003.dat
 18M	rev00004.dat
 18M	rev00005.dat
 19M	rev00006.dat
 18M	rev00007.dat
 19M	rev00008.dat
 18M	rev00009.dat
 19M	rev00010.dat
 18M	rev00011.dat
 18M	rev00012.dat
 18M	rev00013.dat
 19M	rev00014.dat
 18M	rev00015.dat
 19M	rev00016.dat
5.0M	rev00017.dat
2.5G	total

Tested on a Mac OS Extended (HFS+) unencrypted disk image with 3 GB in size:

$ ./bitcoind -datadir=/Volumes/HFS+/
2020-01-09T20:13:51Z UpdateTip: 
new best=00000000000004e055fbb153a6371891f7dc52adbe525b3a5cf3b9413fde7551 
height=189775 version=0x00000001 log2_work=68.414219 tx=5084658 
date='2012-07-19T07:39:48Z' progress=0.010306 cache=285.6MiB(2101977txo)
2020-01-09T20:13:51Z Error: Disk space is too low!

$ cd /Volumes/HFS+/blocks/

$ du -ch *.dat
128M	blk00000.dat
128M	blk00001.dat
128M	blk00002.dat
128M	blk00003.dat
128M	blk00004.dat
128M	blk00005.dat
128M	blk00006.dat
128M	blk00007.dat
128M	blk00008.dat
128M	blk00009.dat
128M	blk00010.dat
128M	blk00011.dat
128M	blk00012.dat
128M	blk00013.dat
128M	blk00014.dat
128M	blk00015.dat
 64M	blk00016.dat
 19M	rev00000.dat
 17M	rev00001.dat
 16M	rev00002.dat
 16M	rev00003.dat
 17M	rev00004.dat
 17M	rev00005.dat
 18M	rev00006.dat
 18M	rev00007.dat
 18M	rev00008.dat
 18M	rev00009.dat
 18M	rev00010.dat
 18M	rev00011.dat
 18M	rev00012.dat
 18M	rev00013.dat
 18M	rev00014.dat
 18M	rev00015.dat
8.0M	rev00016.dat
2.3G	total

eriknylund · 2020-01-09T20:44:27Z

I also built a Bitcoin-Qt.dmg and started a fresh download on machine 1 in the issue report, to verify that I can successfully complete the initial block download on the Mac mini with 480 GB of free disk space at the start. ETA 3 days.

fanquake · 2020-01-10T05:06:14Z

I've just finished a from scratch sync using this PR + 17892, on an macOS (APFS) machine. Data directory is the expected size. No longer seeing very large rev*.dat files, most are between 16 and 21mb.

eriknylund · 2020-01-12T17:24:26Z

I also built a Bitcoin-Qt.dmg and started a fresh download on machine 1 in the issue report, to verify that I can successfully complete the initial block download on the Mac mini with 480 GB of free disk space at the start. ETA 3 days.

I've just finished a from scratch sync using this PR + 17892, on an macOS (APFS) machine. Data directory is the expected size. No longer seeing very large rev*.dat files, most are between 16 and 21mb.

The IBD is now complete and the estimate is spot on. 280 GB of disk space is used. The rev*.dat file sizes are in the range 12-27 MB.

Bitcoin % pwd
/Users/erik/Library/Application Support/Bitcoin

Bitcoin % du -h .
 92M	./blocks/index
276G	./blocks
3.7G	./chainstate
2.0M	./wallets/database
4.0M	./wallets
280G	.

Bitcoin % tail debug.log
2020-01-12T17:17:44Z UpdateTip: 
new best=000000000000000000141d63c0a927235c7e21b39f8c632f9b7e70a9f3fb1659 
height=612543 version=0x3fffe000 log2_work=91.544872 tx=492581053 
date='2020-01-12T17:17:13Z' progress=1.000000 cache=259.8MiB(1712729txo) 
warning='71 of last 100 blocks have unexpected version'

Bitcoin % du -ch blocks/rev*.dat | sort -n
 12M	blocks/rev00311.dat
 12M	blocks/rev01927.dat
 14M	blocks/rev00314.dat
...
 23M	blocks/rev00337.dat
 24M	blocks/rev01037.dat
 27M	blocks/rev01036.dat
 35G	total

eriknylund · 2020-01-12T19:57:46Z

Looking a bit more into the rev files I see what is most likely the truncation issue @kallewoof discovered in #17890

Bitcoin % du -h blocks/rev00337.dat blocks/rev01037.dat blocks/rev01036.dat
 23M	blocks/rev00337.dat
 24M	blocks/rev01037.dat
 27M	blocks/rev01036.dat
Bitcoin % ls -lh blocks/rev00337.dat blocks/rev01037.dat blocks/rev01036.dat
-rw-------  1 erik  staff    21M Jan 10 10:46 blocks/rev00337.dat
-rw-------  1 erik  staff    23M Jan 11 12:56 blocks/rev01036.dat
-rw-------  1 erik  staff    21M Jan 11 12:58 blocks/rev01037.dat

kallewoof · 2020-01-13T04:28:16Z

Thanks! Yeah, we will have to do a one time run-through of all the rev files to clean up the excess use, or we will end up with wasted space for everyone.. I'm honestly not sure why this isn't affecting other systems though. The finalize issue is not mac specific (only the over-preallocating issue is).

kallewoof · 2020-01-13T04:36:07Z

This made me realize that this actually is a bug in the operating system. I couldn't explain why this didn't happen to earlier versions, but I believe I can now.

The bug is that ftruncate does not return pre-allocated disk space.

Before, we would over-preallocate and then immediately truncate back down to the expected size. This worked in pre-APFS land, but APFS has a bug which leaves the pre-allocated space as occupied by the file, even though we call ftruncate.

I will make a minimal test case displaying this behavior and submit a report to Apple.

Edit: https://openradar.appspot.com/radar?id=4930713610616832

Edit: I updated the bug report above with information on F_PUNCHHOLE, which appears to be the way to un-preallocate (return) space; this does nothing, however.

Empact · 2020-01-13T20:33:40Z

src/util/system.cpp

    fst.fst_bytesalloc = 0;
    if (fcntl(fileno(file), F_PREALLOCATE, &fst) == -1) {
        fst.fst_flags = F_ALLOCATEALL;
        fcntl(fileno(file), F_PREALLOCATE, &fst);
    }
-    ftruncate(fileno(file), fst.fst_length);
+    ftruncate(fileno(file), (off_t)offset + length);


nit: could static_cast

Empact · 2020-01-13T20:35:52Z

Code review ACK 5de93dc also checked man fcntl locally

fanquake · 2020-01-14T12:27:50Z

@arvidn Any chance you could comment here, given you patched an APFS related issue in libtorrent some time ago?

arvidn · 2020-01-14T13:17:02Z

This patch changes the behavior of AllocateFileRange(), which is documented to:

this function tries to make a particular range of a file allocated (corresponding to disk space) it is advisory, and the range specified in the arguments will never contain live data

It doesn't seem like this patch correctly implements the documented behavior. It might fix an issue in how AllocateFileRange() is used, but there still seems to be a mismatch in how AllocateFileRange() behaves on other operating systems and what it's documented to do.

If I understand the mailing list post Dominic Giampaolo correctly, in order to implement the documented behavior of AllocateFileRange() you would also need to stat() the file and set:

fst.fst_length = offset + length - st_size;

and if offset + length < st_size, skip the fcntl() call entirely.

kallewoof · 2020-01-14T13:50:20Z

Edit: nevermind, I see what you're saying, but I believe offset is already (supposed to be) equal to st_size, so I don't think it matters.

arvidn · 2020-01-14T13:57:15Z

but I believe offset is already (supposed to be) equal to st_size

Right, this patch makes that assumption. The windows implementation does not make this assumption, nor does the documentation state it will make this assumption.

kallewoof · 2020-01-14T14:13:47Z

I see what you mean. I don't think it matters in this case either way, but it's worth pointing out.

kallewoof · 2020-01-14T14:14:42Z

Either way, that's unrelated to the question which is how to properly pre-allocate (and more importantly, how to return unused space at the end) on APFS.

The macos manpage for fcntl (for F_PEOFPOSMODE) states: > Allocate from the physical end of file. In this case, fst_length indicates the number of newly allocated bytes desired.

bg002h · 2020-01-21T19:10:01Z

My sizes using this pull request, running Catalina and an unencrypted APFS disk:

https://pastebin.com/W53SX4xg

Running master:

https://pastebin.com/0Sj2XXny

All file sizes for both this PR and master don't seem any different while running or after shutdown.

Sjors · 2020-01-21T19:14:52Z

@bg002h ls -alFh won't see the issue. Try du -ch instead.

bg002h · 2020-01-21T19:33:44Z

@Sjors Thx for the tip!

I reformatted my NVME raid to APFS, encrypted, case sensitive since the above.

For master, I see:
https://pastebin.com/GeqsZhmh (rev 50 was 45M)

running this PR (pastern is down):
bcg@Brians-MacBook-Pro blocks % du -ch *
128M blk00000.dat
.
.
.
128M blk00052.dat
16M blk00053.dat
8.0K index
19M rev00000.dat
16M rev00001.dat
16M rev00002.dat
16M rev00003.dat
16M rev00004.dat
17M rev00005.dat
17M rev00006.dat
17M rev00007.dat
18M rev00008.dat
18M rev00009.dat
17M rev00010.dat
17M rev00011.dat
18M rev00012.dat
18M rev00013.dat
17M rev00014.dat
17M rev00015.dat
18M rev00016.dat
17M rev00017.dat
17M rev00018.dat
17M rev00019.dat
17M rev00020.dat
17M rev00021.dat
17M rev00022.dat
17M rev00023.dat
17M rev00024.dat
17M rev00025.dat
17M rev00026.dat
17M rev00027.dat
17M rev00028.dat
17M rev00029.dat
17M rev00030.dat
17M rev00031.dat
17M rev00032.dat
17M rev00033.dat
17M rev00034.dat
17M rev00035.dat
17M rev00036.dat
17M rev00037.dat
17M rev00038.dat
17M rev00039.dat
17M rev00040.dat
17M rev00041.dat
17M rev00042.dat
17M rev00043.dat
17M rev00044.dat
17M rev00045.dat
17M rev00046.dat
17M rev00047.dat
17M rev00048.dat
17M rev00049.dat
17M rev00050.dat
17M rev00051.dat
17M rev00052.dat
2.0M rev00053.dat in progress
7.5G total

bg002h · 2020-01-21T19:42:09Z

using master, and checking du repeatedly during blk1, rev1 gets pretty big before being trimmed back:

cg@Brians-MacBook-Pro blocks % du -ch *
576M blk00000.dat
8.0K index
190M rev00000.dat
766M total

The behavior is not observed using this PR:

bcg@Brians-MacBook-Pro blocks % du -ch *
128M blk00000.dat
8.0K index
17M rev00000.dat
145M total
bcg@Brians-MacBook-Pro blocks % du -ch *
128M blk00000.dat
8.0K index
18M rev00000.dat
146M total
bcg@Brians-MacBook-Pro blocks % du -ch *
128M blk00000.dat
8.0K index
19M rev00000.dat
147M total
bcg@Brians-MacBook-Pro blocks % du -ch *
128M blk00000.dat
8.0K index
19M rev00000.dat
147M total
bcg@Brians-MacBook-Pro blocks % du -ch *
128M blk00000.dat
8.0K index
19M rev00000.dat
147M total
bcg@Brians-MacBook-Pro blocks % du -ch *
128M blk00000.dat
16M blk00001.dat
8.0K index
19M rev00000.dat
1.0M rev00001.dat
164M total
bcg@Brians-MacBook-Pro blocks % du -ch *
128M blk00000.dat
16M blk00001.dat
8.0K index
19M rev00000.dat
1.0M rev00001.dat
164M total
bcg@Brians-MacBook-Pro blocks %
bcg@Brians-MacBook-Pro blocks % du -ch *
128M blk00000.dat
96M blk00001.dat
8.0K index
19M rev00000.dat
12M rev00001.dat
255M total

fjahr · 2020-01-21T20:30:46Z

I ran with --enable-debug though.

Strange, I double checked and still not seeing any large rev files with --enable-debug...

kallewoof · 2020-01-22T01:15:13Z

--enable-debug should not matter in this case.

maaku · 2020-01-22T02:00:51Z

FWIW I'm able to reproduce this on gitian-built clients as far back as 0.12.

laanwj · 2020-01-22T15:46:28Z

code review ACK 75163f4
I have checked the documentation that the new use of F_PREALLOCATE is correct (and the old one is wrong).

Thanks everyone for testing!

75163f4 bug-fix macos: give free bytes to F_PREALLOCATE (Karl-Johan Alm) Pull request description: The macos manpage for `fcntl` (for `F_PEOFPOSMODE`) states: > Allocate from the physical end of file. In this case, fst_length indicates the number of newly allocated bytes desired. This would result in the rev files being essentially pre-allocating 2x their necessary size (this is the case for block files as well, but these are flushed down to their right sizes every time) as they would pre-allocate `pos + length` **free** bytes, rather than allocating `length` bytes after `pos`, as expected. Fixes #17827. ACKs for top commit: eriknylund: ACK 75163f4 built locally. All tests passing. Manual test as per my previous comment above on an older commit, using an APFS unencrypted disk image with 3 GB. laanwj: code review ACK 75163f4 Tree-SHA512: 105c8d56c20acad8febdf0583f1e5721b63376ace325a7a62c2e4b15a442c7131404ed604c32c0cda716791d7ca5aa9f5b6a774ff86e39838bc7e87ca3c42760

The macos manpage for fcntl (for F_PEOFPOSMODE) states: > Allocate from the physical end of file. In this case, fst_length indicates the number of newly allocated bytes desired. Github-Pull: bitcoin#17887 Rebased-From: 75163f4

fanquake · 2020-01-23T00:42:03Z

Being backported to 0.19 in #17988.

daf2fff test: add missing #include to fix compiler errors (Karl-Johan Alm) c8ad23c bug-fix macos: give free bytes to F_PREALLOCATE (Karl-Johan Alm) Pull request description: We're about ready to do a [0.19.1 release](https://github.com/bitcoin/bitcoin/milestone/44); so I've opened this to collect the last remaining backports. If there's something that's been missed / or isn't tagged ["Needs backport (0.19)"](https://github.com/bitcoin/bitcoin/issues?q=is%3Aopen+label%3A%22Needs+backport+%280.19%29%22) that you think should be, please comment. Currently backports: * #17887 - bug-fix macos: give free bytes to F_PREALLOCATE * #17980 - test: add missing #include to fix compiler errors ACKs for top commit: laanwj: ACK daf2fff Tree-SHA512: 8438f992d0c39315a4da4c3f8ab5c92acefada4b0ba5a5cec3775dea2541492d386bf4e7c9e76e1494a6d4cf16a9205287d27ffd23d9c3056f213d733605eeee

75163f4 bug-fix macos: give free bytes to F_PREALLOCATE (Karl-Johan Alm) Pull request description: The macos manpage for `fcntl` (for `F_PEOFPOSMODE`) states: > Allocate from the physical end of file. In this case, fst_length indicates the number of newly allocated bytes desired. This would result in the rev files being essentially pre-allocating 2x their necessary size (this is the case for block files as well, but these are flushed down to their right sizes every time) as they would pre-allocate `pos + length` **free** bytes, rather than allocating `length` bytes after `pos`, as expected. Fixes bitcoin#17827. ACKs for top commit: eriknylund: ACK 75163f4 built locally. All tests passing. Manual test as per my previous comment above on an older commit, using an APFS unencrypted disk image with 3 GB. laanwj: code review ACK 75163f4 Tree-SHA512: 105c8d56c20acad8febdf0583f1e5721b63376ace325a7a62c2e4b15a442c7131404ed604c32c0cda716791d7ca5aa9f5b6a774ff86e39838bc7e87ca3c42760

- [0.19] wallet: Reset reused transactions cache bitcoin#18083 - 0.19: Backports bitcoin#17792 - psbt: handle unspendable psbts bitcoin#17524 - qt: Fix comparison function signature bitcoin#17634 - psbt: check that various indexes and amounts are within bounds bitcoin#17156 - [0.19] psbt: check that various indexes and amounts are within bounds bitcoin#18079 - [0.19] Final backports for 0.19.1 bitcoin#17988 - Bug: IsUsedDestination shouldn't use key id as script id for ScriptHash bitcoin#17924 - qt: Fix deprecated QCharRef usage bitcoin#18101 - gui: Throttle GUI update pace when -reindex bitcoin#18121 - gui: Fix race in WalletModel::pollBalanceChanged bitcoin#18123 - gui: Fix unintialized WalletView::progressDialog bitcoin#18062 - Bugfix: GUI: Hide the HD/encrypt icons earlier so they get re-shown if another wallet is open bitcoin#18007 - bug-fix macos: give free bytes to F_PREALLOCATE bitcoin#17887 - test: add missing #include to fix compiler errors bitcoin#17980 - zmq: Fix due to invalid argument and multiple notifiers bitcoin#17445

kallewoof · 2020-09-02T04:01:24Z

Apple responded to my bug report regarding this, in case people are curious: https://openradar.appspot.com/radar?id=4930713610616832

Summary: The Initial Block Download (IBD) on macOS Catalina uses a lot more disk space than expected. The macos manpage for fcntl (for F_PEOFPOSMODE) states: > Allocate from the physical end of file. In this case, fst_length indicates the number of newly allocated bytes desired. This would result in the rev files being essentially pre-allocating 2x their necessary size (this is the case for block files as well, but these are flushed down to their right sizes every time) as they would pre-allocate pos + length free bytes, rather than allocating length bytes after pos, as expected. Note: more explanations on the issue from Apple in this bug report https://openradar.appspot.com/radar?id=4930713610616832 Backport of Bitcoin Core [[bitcoin/bitcoin#17887 | PR17887]] Test Plan: `ninja check` to verify that the patch does not accidentaly break anything else. But I do not have a MacOS to run the tests on, and try to replicate the issue. Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Subscribers: deadalnix Differential Revision: https://reviews.bitcoinabc.org/D7526

75163f4 bug-fix macos: give free bytes to F_PREALLOCATE (Karl-Johan Alm) Pull request description: The macos manpage for `fcntl` (for `F_PEOFPOSMODE`) states: > Allocate from the physical end of file. In this case, fst_length indicates the number of newly allocated bytes desired. This would result in the rev files being essentially pre-allocating 2x their necessary size (this is the case for block files as well, but these are flushed down to their right sizes every time) as they would pre-allocate `pos + length` **free** bytes, rather than allocating `length` bytes after `pos`, as expected. Fixes bitcoin#17827. ACKs for top commit: eriknylund: ACK 75163f4 built locally. All tests passing. Manual test as per my previous comment above on an older commit, using an APFS unencrypted disk image with 3 GB. laanwj: code review ACK 75163f4 Tree-SHA512: 105c8d56c20acad8febdf0583f1e5721b63376ace325a7a62c2e4b15a442c7131404ed604c32c0cda716791d7ca5aa9f5b6a774ff86e39838bc7e87ca3c42760

daf2fff test: add missing #include to fix compiler errors (Karl-Johan Alm) c8ad23c bug-fix macos: give free bytes to F_PREALLOCATE (Karl-Johan Alm) Pull request description: We're about ready to do a [0.19.1 release](https://github.com/bitcoin/bitcoin/milestone/44); so I've opened this to collect the last remaining backports. If there's something that's been missed / or isn't tagged ["Needs backport (0.19)"](https://github.com/bitcoin/bitcoin/issues?q=is%3Aopen+label%3A%22Needs+backport+%280.19%29%22) that you think should be, please comment. Currently backports: * bitcoin#17887 - bug-fix macos: give free bytes to F_PREALLOCATE * bitcoin#17980 - test: add missing #include to fix compiler errors ACKs for top commit: laanwj: ACK daf2fff Tree-SHA512: 8438f992d0c39315a4da4c3f8ab5c92acefada4b0ba5a5cec3775dea2541492d386bf4e7c9e76e1494a6d4cf16a9205287d27ffd23d9c3056f213d733605eeee

fanquake added the macOS label Jan 7, 2020

kallewoof mentioned this pull request Jan 7, 2020

Non-latest rev files are sometimes re-opened but never re-flushed #17890

Closed

kallewoof mentioned this pull request Jan 8, 2020

bug-fix: delay flushing undo files until after they are finalized #17892

Closed

fanquake added the Needs backport (0.19) label Jan 8, 2020

Empact reviewed Jan 13, 2020

View reviewed changes

kallewoof force-pushed the macos-f_preallocate_fix branch from 5de93dc to 2db88bd Compare January 14, 2020 02:02

bug-fix macos: give free bytes to F_PREALLOCATE

75163f4

The macos manpage for fcntl (for F_PEOFPOSMODE) states: > Allocate from the physical end of file. In this case, fst_length indicates the number of newly allocated bytes desired.

kallewoof force-pushed the macos-f_preallocate_fix branch from 2db88bd to 75163f4 Compare January 14, 2020 23:57

laanwj merged commit 75163f4 into bitcoin:master Jan 22, 2020

fanquake mentioned this pull request Jan 23, 2020

[0.19] Final backports for 0.19.1 #17988

Merged

fanquake removed the Needs backport (0.19) label Jan 23, 2020

MarkLTZ mentioned this pull request Apr 4, 2020

Bitcoin PR tracking litecoinz-core/litecoinz#14

Closed

jolonf mentioned this pull request May 5, 2020

macOS: rev*.dat files 10x larger disk usage than file size on APFS bitcoin-sv/bitcoin-sv#171

Closed

kallewoof deleted the macos-f_preallocate_fix branch September 2, 2020 04:00

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bug-fix macos: give free bytes to F_PREALLOCATE #17887

bug-fix macos: give free bytes to F_PREALLOCATE #17887

kallewoof commented Jan 7, 2020 •

edited

Loading

sipa commented Jan 7, 2020

fanquake commented Jan 7, 2020

kallewoof commented Jan 7, 2020 •

edited

Loading

kallewoof commented Jan 7, 2020 •

edited

Loading

DrahtBot commented Jan 7, 2020 •

edited

Loading

kallewoof commented Jan 7, 2020

IPGlider commented Jan 7, 2020

fanquake commented Jan 8, 2020

eriknylund commented Jan 9, 2020

eriknylund commented Jan 9, 2020

fanquake commented Jan 10, 2020

eriknylund commented Jan 12, 2020 •

edited

Loading

eriknylund commented Jan 12, 2020

kallewoof commented Jan 13, 2020

kallewoof commented Jan 13, 2020 •

edited

Loading

Empact Jan 13, 2020

kallewoof Jan 14, 2020

Empact commented Jan 13, 2020

fanquake commented Jan 14, 2020

arvidn commented Jan 14, 2020 •

edited

Loading

kallewoof commented Jan 14, 2020 •

edited

Loading

arvidn commented Jan 14, 2020

kallewoof commented Jan 14, 2020

kallewoof commented Jan 14, 2020

bg002h commented Jan 21, 2020

Sjors commented Jan 21, 2020

bg002h commented Jan 21, 2020

bg002h commented Jan 21, 2020

fjahr commented Jan 21, 2020

kallewoof commented Jan 22, 2020

maaku commented Jan 22, 2020

laanwj commented Jan 22, 2020 •

edited

Loading

fanquake commented Jan 23, 2020

kallewoof commented Sep 2, 2020

bug-fix macos: give free bytes to F_PREALLOCATE #17887

bug-fix macos: give free bytes to F_PREALLOCATE #17887

Conversation

kallewoof commented Jan 7, 2020 • edited Loading

sipa commented Jan 7, 2020

fanquake commented Jan 7, 2020

kallewoof commented Jan 7, 2020 • edited Loading

kallewoof commented Jan 7, 2020 • edited Loading

DrahtBot commented Jan 7, 2020 • edited Loading

Conflicts

kallewoof commented Jan 7, 2020

IPGlider commented Jan 7, 2020

fanquake commented Jan 8, 2020

eriknylund commented Jan 9, 2020

eriknylund commented Jan 9, 2020

fanquake commented Jan 10, 2020

eriknylund commented Jan 12, 2020 • edited Loading

eriknylund commented Jan 12, 2020

kallewoof commented Jan 13, 2020

kallewoof commented Jan 13, 2020 • edited Loading

Empact Jan 13, 2020

Choose a reason for hiding this comment

kallewoof Jan 14, 2020

Choose a reason for hiding this comment

Empact commented Jan 13, 2020

fanquake commented Jan 14, 2020

arvidn commented Jan 14, 2020 • edited Loading

kallewoof commented Jan 14, 2020 • edited Loading

arvidn commented Jan 14, 2020

kallewoof commented Jan 14, 2020

kallewoof commented Jan 14, 2020

bg002h commented Jan 21, 2020

Sjors commented Jan 21, 2020

bg002h commented Jan 21, 2020

bg002h commented Jan 21, 2020

fjahr commented Jan 21, 2020

kallewoof commented Jan 22, 2020

maaku commented Jan 22, 2020

laanwj commented Jan 22, 2020 • edited Loading

fanquake commented Jan 23, 2020

kallewoof commented Sep 2, 2020

kallewoof commented Jan 7, 2020 •

edited

Loading

kallewoof commented Jan 7, 2020 •

edited

Loading

kallewoof commented Jan 7, 2020 •

edited

Loading

DrahtBot commented Jan 7, 2020 •

edited

Loading

eriknylund commented Jan 12, 2020 •

edited

Loading

kallewoof commented Jan 13, 2020 •

edited

Loading

arvidn commented Jan 14, 2020 •

edited

Loading

kallewoof commented Jan 14, 2020 •

edited

Loading

laanwj commented Jan 22, 2020 •

edited

Loading