out_s3: daemon thread stability refactor rebased on top of AWS distro 2.31.10#24
out_s3: daemon thread stability refactor rebased on top of AWS distro 2.31.10#24PettitWesley wants to merge 3 commits into2_31_10_all_cherry_picksfrom
Conversation
…g line Signed-off-by: Wesley Pettit <wppttt@amazon.com>
|
@mathewfala the diff in the PR on upstream is misleading since our distro has diverged. This is the S3 changes that I'm testing/reviewing on top of our 2.31.10 all cherry picks. |
PettitWesley
changed the title
|
|
||
| /* on startup, we only send old chunks in this routine */ | ||
| if (is_startup == FLB_TRUE && fs_stream == ctx->stream_active) { | ||
| flb_info("put_all_chunks: stream_active has %d chunks", mk_list_size(&fs_stream->files)); |
There was a problem hiding this comment.
remember to remove log for testing purposes
| /* data was sent successfully- delete the local buffer */ | ||
| if (chunk->locked == FLB_TRUE) { | ||
| /* remove from upload_queue */ | ||
| if (chunk->_head.next != NULL && chunk->_head.prev != NULL) { |
There was a problem hiding this comment.
I think there's a mk_list function for this check
Signed-off-by: Wesley Pettit <wppttt@amazon.com>
…clay's PR Signed-off-by: Wesley Pettit <wppttt@amazon.com>
| * Puts an event on the event | ||
| * loop which will wake this coro back up | ||
| */ | ||
| flb_time_sleep(ctx->timer_ms); |
There was a problem hiding this comment.
For the final version I think I may need to set this to be some static number that's always less than the Grace setting. Some fraction of it. Otherwise the sleep here can block shutdown until it resumes and eat up time that could be used to send data on shutdown.
| if (ret < 0) { | ||
| /* re-add chunk to list */ | ||
| if (chunk) { | ||
| s3_store_file_unlock(chunk); |
There was a problem hiding this comment.
This got re-added by my rebase and needs to be removed otherwise it will crash...
| m_upload->upload_errors += 1; | ||
| /* re-add chunk to list */ | ||
| if (chunk) { | ||
| s3_store_file_unlock(chunk); |
There was a problem hiding this comment.
remove unlock from s3_store.c, its not needed and shouldn't be a function anymore
| @@ -1245,10 +1285,41 @@ static int put_all_chunks(struct flb_s3 *ctx) | |||
| if (ret < 0) { | |||
| s3_store_file_unlock(chunk); | |||
| int size_check = FLB_FALSE; | ||
| int part_num_check = FLB_FALSE; | ||
| int timeout_check = FLB_FALSE; | ||
| time_t create_time; |
There was a problem hiding this comment.
comment above is wrong:
* return value is one of 0, -1, -2.
- 0 means uploaded successfully.
- -1 means failed to upload data, will retry.
- -2 means failed to upload data, but can't retry because reach the retry_limit.
The may13th one commit branch has most up to date changes that need to be sync'd here.
There was a problem hiding this comment.
Return Value is wrong here too. Another issue from rebase.
PettitWesley
left a comment
PettitWesley
left a comment
There was a problem hiding this comment.
I need to sync all changes here from branch: s3-refactor-aws-distro-may13
There were more issues from rebasing which I found when I ran the file here through a diff against the one on upstream github (rebased on top of a different set of commits).
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
1 participant
…g line
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:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
Documentation
Backporting
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.