Skip to content

S3 bugfix and refactor#5257

Merged
therandomstring merged 11 commits intosyslog-ng:masterfrom
therandomstring:s3-bugfix
Mar 19, 2025
Merged

S3 bugfix and refactor#5257
therandomstring merged 11 commits intosyslog-ng:masterfrom
therandomstring:s3-bugfix

Conversation

@therandomstring
Copy link
Contributor

This pull request:

  • Fixes a bug, where a sustained, high EPS upload would result in substantial data loss with max_object_size set to reasonably low values and upload_threads set to anything other then 1. This bug could also have intermittently manifested with max_object_size at higher values. This bug could not have been fixed with the current code structure, necessitating a refactor.
  • Also contains a bugfix for syslog-ng potentially crashing when stopped by SIGINT while a python destination is actively writing logs.
  • Contains a general refactor for:
    • Ease of maintenance
    • Better utilising existing boto3 functionality
  • Introduces a new option to add a suffix to the object key.

New options:

  • object_key_suffix: This new option allows for a suffix after the multipart index. Useful for specifying file extension, thus its default value is .log

❗ ❗ ❗ This pull request introduces two important changes: ❗ ❗ ❗

  • Due to the new default file extension the multipart index is no longer positioned at the end of the object key. This could break some end user scripts.
  • The multipart uploads are now handled by boto3. As a consequence
    • The upload_threads option now governs the per-upload thread usage. This means the syslog-ng S3 destination driver now uses maximum (upload threads * max pending uploads) + 1 threads, opposed to upload_threads + 1. Users with high python thread count should adjust their configuration accordingly.
    • Although the refactored destination should not increase network traffic, a higher peak load might be experienced.

…hon destination were to be writing a log simultaneously

Signed-off-by: Bálint Horváth <[email protected]>
The backfill behaviour will be replaced with a simpler solution in a later commit on this branch.

Signed-off-by: Bálint Horváth <[email protected]>
@kira-syslogng
Copy link
Contributor

Build FAILURE

4 similar comments
@kira-syslogng
Copy link
Contributor

Build FAILURE

@kira-syslogng
Copy link
Contributor

Build FAILURE

@kira-syslogng
Copy link
Contributor

Build FAILURE

@kira-syslogng
Copy link
Contributor

Build FAILURE

@kira-syslogng
Copy link
Contributor

Build FAILURE

1 similar comment
@kira-syslogng
Copy link
Contributor

Build FAILURE

@kira-syslogng
Copy link
Contributor

Build FAILURE

therandomstring added a commit to therandomstring/syslog-ng that referenced this pull request Mar 12, 2025
@kira-syslogng
Copy link
Contributor

Build FAILURE

@therandomstring therandomstring marked this pull request as ready for review March 12, 2025 14:35
@therandomstring
Copy link
Contributor Author

@kovgeri01 I reworked the locks. Could you do another review round with them in mind?

These introduce simpler, easier to maintain replacements to the S3Object class. The new classes also help with separation of concern.

Signed-off-by: Bálint Horváth <[email protected]>
Some dependencies (however unlikely) may not be installed, resulting in syslog-ng crashing. This should eliminate that concern.

Signed-off-by: Bálint Horváth <[email protected]>
This commit temporarily disables the upload-on-close behaviour. It will be reintroduced later.

Signed-off-by: Bálint Horváth <[email protected]>
… and shutdown

This commit reintroduces the removed upload-on-close behaviour and replaces the old backfill.

Signed-off-by: Bálint Horváth <[email protected]>
@kira-syslogng
Copy link
Contributor

Build FAILURE

@therandomstring therandomstring merged commit 7dd4145 into syslog-ng:master Mar 19, 2025
28 of 29 checks passed
therandomstring added a commit to therandomstring/syslog-ng that referenced this pull request Mar 20, 2025
Signed-off-by: Bálint Horváth <[email protected]>
(cherry picked from commit 7dd4145)
HofiOne pushed a commit to HofiOne/syslog-ng that referenced this pull request Mar 20, 2025
HofiOne added a commit to HofiOne/syslog-ng that referenced this pull request Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants