-
Notifications
You must be signed in to change notification settings - Fork 2
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
S3 refactor timer coros => change to store timer coros on flb_sched struct #28
base: s3-refactor-timer-coros-awspr
Are you sure you want to change the base?
S3 refactor timer coros => change to store timer coros on flb_sched struct #28
Conversation
…t async timers on the sched Signed-off-by: Wesley Pettit <wppttt@amazon.com>
Signed-off-by: Wesley Pettit <wppttt@amazon.com>
c20a8f9
to
8576725
Compare
void *sched; | ||
struct flb_sched *sched; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonder why this is a void *
in the first place. Seems like all pointers are void *
in this file. I'd keep it void *
unless we understand this convention and have a reason to break it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
void * is useful if at compile time it could store multiple different structs. Here its always one struct, so there's no good reason AFAIK for it to be void.
/* Each event loop has a scheduler instance */ | ||
struct flb_sched *sched; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling out that this is the central change in this pr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look great! There may be a typo here: flb_async_timer_cleanup(config->sched->async_timer_list_destroy)
as it may need to be (config->sched)
@@ -929,7 +916,7 @@ int flb_engine_start(struct flb_config *config) | |||
flb_net_dns_lookup_context_cleanup(&dns_ctx); | |||
flb_sched_timer_cleanup(config->sched); | |||
flb_upstream_conn_pending_destroy_list(&config->upstreams); | |||
flb_output_async_timer_cleanup(config); | |||
flb_async_timer_cleanup(config->sched->async_timer_list_destroy); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo? Should this be config->sched
mk_list_foreach_safe(head, tmp, destroy_list) { | ||
async_timer = mk_list_entry(head, struct flb_out_async_timer, _head); | ||
async_timer = mk_list_entry(head, struct flb_async_timer, _head); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: need to acquire mutex every time the engine reads or the worker thread writes/deletes.
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. |
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.