Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

disk-buffer: add prealloc() option, and change defaults in 4.0 #4056

Merged
merged 11 commits into from Dec 4, 2022
Merged

disk-buffer: add prealloc() option, and change defaults in 4.0 #4056

merged 11 commits into from Dec 4, 2022

Conversation

alltilla
Copy link
Collaborator

@alltilla alltilla commented Jun 24, 2022 •
edited

TODO:

  • include the change in the 4.0 news file PR.

Signed-off-by: Attila Szakacs attila.szakacs@oneidentity.com

@kira-syslogng
Copy link
Contributor

Build SUCCESS

@alltilla alltilla added this to the syslog-ng-4.0 milestone Jun 24, 2022
@MrAnno
Copy link
Collaborator

MrAnno commented Jun 24, 2022

Can we also pre-allocate the full disk-buffer size in case truncation is completely disabled?

@alltilla
Copy link
Collaborator Author

Yes we can, I added that, too.

@alltilla alltilla changed the title disk-buffer: disable truncation by default in 4.0 disk-buffer: disable truncation by default & prealloc in 4.0 Jun 24, 2022
MrAnno
MrAnno reviewed Jun 24, 2022
View reviewed changes
modules/diskq/diskq.c Outdated Show resolved Hide resolved
modules/diskq/qdisk.c Outdated Show resolved Hide resolved
@alltilla alltilla force-pushed the 4.0-disk-buffer-truncate-disable branch from 02e39e5 to ce10a10 Compare July 1, 2022 08:05
@alltilla alltilla changed the title disk-buffer: disable truncation by default & prealloc in 4.0 disk-buffer: add prealloc() option, and change defaults in 4.0 Jul 1, 2022
Kokan
Kokan reviewed Jul 1, 2022
View reviewed changes
modules/diskq/qdisk.c Outdated Show resolved Hide resolved
modules/diskq/diskq.c Outdated Show resolved Hide resolved
@alltilla alltilla force-pushed the 4.0-disk-buffer-truncate-disable branch from ce10a10 to e2de300 Compare July 1, 2022 10:59
@alltilla
Copy link
Collaborator Author

alltilla commented Jul 1, 2022

It seems like macOS does not have posix_fallocate() :(

alltilla added a commit to alltilla/syslog-ng that referenced this pull request Jul 1, 2022
@alltilla
news: add entry for syslog-ng#4056
41d6724 
Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
@alltilla alltilla force-pushed the 4.0-disk-buffer-truncate-disable branch from e2de300 to 41d6724 Compare July 1, 2022 13:28
@alltilla
Copy link
Collaborator Author

alltilla commented Jul 1, 2022 •
edited

Added a hand-written preallocation for platforms, which do not have posix_fallocate().

@MrAnno
Copy link
Collaborator

MrAnno commented Jul 1, 2022

I'm just wondering, how long it might take to allocate 100-1000 gigabytes of storage with posix_fallocate on an average HDD.
Hopefully, this fallocate version won't take minutes, but the compat version actually writes the disk, which could take a lot of time at startup.

@alltilla
Copy link
Collaborator Author

alltilla commented Jul 4, 2022

We can decide to not support preallocation at all on platforms, where we cannot guarantee it to be relatively fast.

@Kokan
Copy link
Collaborator

Kokan commented Jul 4, 2022

Please also note that posix_fallocate where it implemented, it does not mean it is fast/efficient, as if fallocate is not supported it falls back to a code that writes the zeroes (just like your compat solution)(see notes for manpage). It is not only platform dependent, on linux (and possible other unix) it is also file system dependent (like fat32 does not support fallocate).

If we want to avoid slow operation of pre-allocation we should check if fallocate present in compile time, and also check runtime the result of fallocate and both case fall back not to pre-allocate (I would even say not to truncate the file size.) So it would work like pre-allocation turned off.

@alltilla
Copy link
Collaborator Author

On a HDD with ext4, the preallocation takes milliseconds.
On a HDD with NTFS it is really slow (10 GiB took ~5 minutes), and cannot be cancelled, due to my dumb implementation.

@alltilla
Copy link
Collaborator Author

alltilla commented Oct 6, 2022

prealloc(): We will make the default no, there will be no auto for now.

alltilla added a commit to alltilla/syslog-ng that referenced this pull request Nov 5, 2022
@alltilla
news: add entry for syslog-ng#4056
70d1caf 
Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
@alltilla alltilla force-pushed the 4.0-disk-buffer-truncate-disable branch from 41d6724 to 70d1caf Compare November 5, 2022 10:22
@alltilla
Copy link
Collaborator Author

alltilla commented Nov 5, 2022

Updated as we discussed. This is now ready for review.

MrAnno
MrAnno reviewed Nov 9, 2022
View reviewed changes
modules/diskq/diskq.c Outdated Show resolved Hide resolved
alltilla added a commit to alltilla/syslog-ng that referenced this pull request Nov 12, 2022
@alltilla
news: add entry for syslog-ng#4056
61fb112 
Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
@alltilla alltilla force-pushed the 4.0-disk-buffer-truncate-disable branch from 70d1caf to 61fb112 Compare November 12, 2022 13:53
alltilla added a commit to alltilla/syslog-ng that referenced this pull request Nov 12, 2022
@alltilla
news: add entry for syslog-ng#4056
5b9a385 
Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
@alltilla alltilla force-pushed the 4.0-disk-buffer-truncate-disable branch from 61fb112 to 5b9a385 Compare November 12, 2022 14:04
alltilla added a commit to alltilla/syslog-ng that referenced this pull request Nov 12, 2022
@alltilla
news: add entry for syslog-ng#4056
ee2d66f 
Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
alltilla and others added 11 commits December 4, 2022 14:58
@alltilla
disk-buffer: disable truncation by default in 4.0
f73b594 
Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
@alltilla
disk-buffer: add prealloc() option to preallocate new disk-buffer files
d21fc48 
Its default is no in 3.x and yes in 4.x.

Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
@alltilla
disk-buffer: add compat function for preallocation
b741c08 
Some platforms do not have posix_fallocate() (e.g. macOS).

Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
@alltilla
disk-buffer: enrich error messages in _attach()
a92194e 
Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
@alltilla
disk-buffer: disallow enabling truncation while preallocation is enabled
68604d4 
Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
@alltilla
disk-buffer: rename _truncate_file_to_minimal() to _maybe_truncate_...()
24cfc81 
It does not always truncate the file.

Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
@alltilla
disk-buffer: refactor diskq-config
3099e20 
After this change we store in the singleton whether the user set the
global config values manually, or we are using the hard-coded defaults.

Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@alltilla
disk-buffer: add disk_queue_config_is_xyz_set_explicitly()
0069e73 
Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@alltilla
disk-buffer: factor out _set_truncate_size_ratio_and_prealloc()
2a0f17a 
This will become complex, so better move it to a separate function.

Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@alltilla
disk-buffer: disable truncation or preallocation, if needed and possible
b4e327d 
It is needed when they are both enabled.
It is possible if one of them is not set explicitly by the user.

Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@alltilla
news: add entry for syslog-ng#4056
7ea05be 
Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
@alltilla alltilla force-pushed the 4.0-disk-buffer-truncate-disable branch from 604060e to 7ea05be Compare December 4, 2022 14:00
@alltilla
Copy link
Collaborator Author

alltilla commented Dec 4, 2022

Done!

@MrAnno
Copy link
Collaborator

MrAnno commented Dec 4, 2022

@kira-syslogng do stresstest

@kira-syslogng
Copy link
Contributor

Kira-stress-test: Build FAILURE

@MrAnno
Copy link
Collaborator

MrAnno commented Dec 4, 2022

(Stress test is out of free disk space)

@MrAnno MrAnno merged commit ab63ada into syslog-ng:master Dec 4, 2022
bazsi added a commit that referenced this pull request Dec 5, 2022
@bazsi
NEWS.md: updated with #4056
2bea4b2 
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
@bazsi bazsi mentioned this pull request Dec 5, 2022
Merged
bazsi added a commit that referenced this pull request Dec 5, 2022
@bazsi
NEWS.md: updated with #4056
db9dc82 
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
alltilla pushed a commit that referenced this pull request Dec 5, 2022
@bazsi @alltilla
NEWS.md: updated with #4056
f331217 
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
MrAnno pushed a commit that referenced this pull request Dec 5, 2022
@bazsi @MrAnno
NEWS.md: updated with #4056
86df0b1 
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
MrAnno pushed a commit that referenced this pull request Dec 5, 2022
@bazsi @MrAnno
NEWS.md: updated with #4056
310f99e 
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
@kira-syslogng
Copy link
Contributor

Kira-stress-test: Build FAILURE

bazsi added a commit that referenced this pull request Dec 8, 2022
@bazsi
NEWS.md: updated with #4056
365027a 
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
bazsi added a commit that referenced this pull request Dec 10, 2022
@bazsi
NEWS.md: updated with #4056
8459dd8 
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
bazsi added a commit that referenced this pull request Dec 15, 2022
@bazsi
NEWS.md: updated with #4056
071a329 
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
alltilla pushed a commit that referenced this pull request Dec 15, 2022
@bazsi @alltilla
NEWS.md: updated with #4056
1a81a02 
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kokan Kokan Kokan left review comments

@MrAnno MrAnno MrAnno approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
syslog-ng-4.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@alltilla @kira-syslogng @MrAnno @Kokan