[release/6.0] Align preallocationSize behavior #59078
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
* file length should always be set * file content should always be zeroed * OpenOrCreate can't shrink an existing file
…t posix_fallocate
Co-authored-by: Stephen Toub <stoub@microsoft.com>
|
Tagging subscribers to this area: @dotnet/area-system-io Issue DetailsBackport of #58726 to release/6.0 /cc @adamsitnik Customer ImpactTestingRisk
|
|
cc @danmoseley |
|
Trying to better understand the customer impact of this
is that bad? i'm not clear on how this differs with Unix. is the situation here that there is not enough space on disk, and on Unix that would throw immediately whereas on Windows not necessarily? Still I'm inlined to OK this as it seems to get us to a state that's easier for us and customers to reason about. @stephentoub could you give a 2nd opinion here, particularly on the risk? |
|
@steveisok the tvOS and iOS builds took over 180 mins (although, it seems at least one was on the verge of complete). Is this a known issue? I will restart as our understanding is that the runtime-staging /build/ must succeed. |
Basically if you do: new FileStream(..., preallocationSize: 1MB).Dispose();without this change, on Windows you'll get a 0-length file and on Unix you'll get a 1MB file filled with 0s. With this change, you'll get 1MB-filled-with-0s for both.
I prefer the Windows behavior, but we apparently lack a good, fast way to achieve that on Unix (@tmds may be able to confirm). For existing .NET Framework APIs, it's one thing to have a difference in behavior across platforms, but for new functionality, we should strive to avoid that whenever possible, so I do think we should align the behaviors, and if there's no way to align to the Windows behavior on Unix, that leaves either aligning to the Unix behavior on Windows or removing the feature entirely. Given that, this seems reasonable. And if we're going to do it, we should do it in 6.0, as otherwise we'll be introducing a breaking change in 7.0 on Windows. |
On Linux, you can Whether the file length changes is important. For example, if the user is going to append, probably he doesn't want the file length to change. If this is an intended use-case, it may be better to ignore |
|
Thanks, @tmds. @adamsitnik, I agree with Tom; if we can get the desired behavior on both Windows and Linux, let's do so and just ignore the flag everywhere else. At that point, the preallocationSize is what it was intended to be, purely a performance hint. |
stephentoub
added
the
NO-MERGE
@danmoseley We should see less timeouts as a result of backporting #59154 There are infra issues lingering (hosted pool performance & helix upload times), but since Alex's PR reduces the times of the build step, we should see less timeouts. |
danmoseley
removed
the
Servicing-consider
The other goal of this feature was increased reliability - if I can't allocate a file of a given size, I want to fail fast (i.e. when copying two files, we would know it up-front that we can't store file of a given size, not in the middle when a write operation fail or at the end when close fails). The problem is that It seems that I've failed to convince you that it's worth to include this in 6.0. Since I am starting my 2 week of vacations now, I am going to close the PR. Thanks! |
I think it's problem that 6.0 differs in behavior between Windows and Unix.
I don't understand. The only thing that prevents us for having good, consistent behavior everywhere is wasm? That should not stop us. |
|
Reopening per @stephentoub comment. @dotnet/area-system-io (w/o Adam) can you please advise whether there's a 6.0 change needed and if so someone needs to prepare it if it isn't this change? |
|
@danmoseley we must probably need to prepare another PR to align the behaviors the other way around (make Unix behave as Windows using @tmds' suggestion). |
|
Presumably if we do it we have to do it now as a change later would be breaking. |
|
I would propose to merge this PR, implement the "align Unix to Windows" PR in 7.0 and then port it to 6. If for some reason we fail to do the later, we will still have behavior alignment, that's better than no alignment. However, if we ship this behavior in 6 (the one implemented in this PR), it would be a breaking change if we flip it in 7. |
|
I can make a PR on Monday. |
|
Closing in favor of Tom's PRs. |
ghost
locked as resolved and limited conversation to collaborators
Backport of #58726 to release/6.0
/cc @adamsitnik
Customer Impact
In #51111 (.NET 6 preview 6), we have added the possibility to specify
preallocationSizewhen creating a file. When the file is being created, we ask the OS to allocate file of a given size. If there is not enough free space or the given File System does not support files of a given size, an exception is thrown.While using this feature in #58167 @stephentoub raised concern (#58167 (comment)) about implementation differences between the OSes.
Namely: on Windows, the OS would preallocate file of a given size, but not update EOF immediately (end of file, reported by
FileStream.Length). If the user would write less than initially requested, the EOF would be equal to the actual number of written bytes.Having such a difference would mean this feature is broken as the behavior would differ across OSes.
The behavior has been aligned, and now for Windows and Unix the file is preallocated and EOF is set immediately.
Moreover, a macOS bug was fixed when for file opened with
FileMode.OpenOrCreateandpreallocationSize < EOFthe file could get shrunk.Testing
The change was tested using automated and manual testing, including trying to create a file larger than available space and file larger than supported by given File System.
Risk
The risk is low as this was relatively niche feature added recently.