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.

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

input_chunk: express specific errors using negative encoded numerical error values. #9919

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pwhelan
Copy link
Contributor

@pwhelan pwhelan commented Feb 5, 2025

Summary

Change some internal flb_input_chunk APIs to return a negative value encoding specific errno codes in them instead of just return -1.

The semantics of the calls should not change but it does force callers of the relevant functions to check for negative values instead of -1.

This PR also has a fix for in_emitter to accept negative values as errors instead of just the value -1.

Description

This PR includes everal changes to the flb_input_chunk API so it is possible to know the nature of the error when a call to flb_log_append fails.

There is a refactor of the function input_chunk_get to return a negative value encoded with an errno number to specify the following possible errors:

  • ENOMEM: unable to allocate memory.
  • ENOSPC: have reached a space limit.
  • EAGAIN: buffer is paused, try again.
  • EIO: I/O error when creating a chunk.

The function input_chunk_append_raw has been refactored to return the negative value of errno if it is set during the call if there is an error or the negative value of EIO if not.

These changes are necessary for a forth-coming change in in_http which uses these errors and others to generate more verbose HTTP error response codes.


Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • Example configuration file for the change
  • Debug log output from testing the change
  • Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • Run local packaging test showing all targets (including any new ones) build.
  • Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • Documentation required for this feature

Backporting

  • Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

@pwhelan pwhelan changed the title Pwhelan ret values input chunk input_chunk: express specific errors using negative encoded numerical error values. Feb 5, 2025
Copy link
Collaborator

@niedbalski niedbalski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need more information

@@ -237,6 +237,9 @@ static int in_emitter_ingest_ring_buffer(struct flb_input_instance *in,
flb_sds_destroy(ec.tag);
msgpack_sbuffer_destroy(&ec.mp_sbuf);
}
if (ret < 0) {
return -1;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the two conditions, why not return in the previous < 0?

Copy link
Contributor Author

@pwhelan pwhelan Feb 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I was trying to avoid problems where the buffer would not be drained if there was a single append that caused an error.

I am still thinking if the approach of halting the draining of the ring buffer is the right approach. @edsiper any thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been looking at this and pondering what to do about it. This code could wind up dropping ring buffers if flb_log_append ever throws an error as it is. Fixing this seems out of scope for this PR but we should tackle it. I actually started working on a solution that uses lwrb_peek and lwrb_skip to leave the ring buffer untouched when flb_log_append returns an error.

…rno.

Not all errors will be the same es the errno values and this code does not set
the actuall errno variable.

Signed-off-by: Phillip Adair Stewart Whelan <[email protected]>
…k return codes.

Signed-off-by: Phillip Adair Stewart Whelan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants