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

Output reloading: ensure all events get eventually published #17657

Closed

Conversation

ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Apr 10, 2020

NOTE: This PR is VERY MUCH a WIP. It is not intended to be reviewed at this point. At this point, it is primarily being used for debugging the event publishing flow with output reloading.

What does this PR do?

This PR adds a unit test for output reloading. The test reloads the output multiple times, very quickly, and checks if all events are eventually published.

Why is it important?

If the output is reloaded multiple times very quickly, not all events seem to get published eventually. This PR adds a test that proves it so we can then work towards a fix.

fmt.Println("reloading output...")
pipeline.output.Set(out)

//time.Sleep(4 * time.Millisecond)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interestingly, if some time is allowed between output reloads the chances of all events eventually being published are MUCH higher. Obviously this is not a solution, just an observation that might give a clue as to what's happening.

@ycombinator
Copy link
Contributor Author

Based on debugging statements I just added, it appears that sometimes the memory queue consumer is returning events but the output worker is somehow not getting them for publishing. This is causing the test to fail (eventually all events are not being published).

Investigating deeper...

@andresrc andresrc added [zube]: Inbox Team:Services (Deprecated) Label for the former Integrations-Services team labels Apr 13, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations-services (Team:Services)

@ycombinator ycombinator force-pushed the lb-output-reloading-unit-test branch 2 times, most recently from b124ca3 to dabff5e Compare April 13, 2020 20:54
@ycombinator
Copy link
Contributor Author

Discussed the test and experimental code changes made in this PR with @urso off-PR. We decided to "move" the test implemented in this PR into #17381, as the latter PR contains some code changes that might help make this test pass. Some of the experimental code changes made in this PR here might also need to be moved over to help make the test pass.

So I've closed this PR unmerged and will continue further development in #17381.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Services (Deprecated) Label for the former Integrations-Services team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants