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

Bugfix/disconnect due to slow consumer event drop #289

Merged

Conversation

kashyap-splunk
Copy link
Collaborator

Adding mechanism to drop events if the events queue is full to avoid the blocking of the main nozzle goroutine and hence avoid passing the back pressure back to Doppler when the downstream is slow.

There is still room to further improve it by moving the event parsing from main thread to writer threads. This will be addressed in later pull-requests targeted in subsequent releases.

@kashyap-splunk kashyap-splunk marked this pull request as ready for review September 20, 2021 15:22
@luckyj5
Copy link
Collaborator

luckyj5 commented Sep 20, 2021

How far did we test it? Did we test it with reproducible scenario with slow-consumers errors on tc?

@kashyap-splunk
Copy link
Collaborator Author

How far did we test it? Did we test it with reproducible scenario with slow-consumers errors on tc?

Yes, tested with a low values of the queue size and HEC workers to reproduce the errors while reading and verified that the new build drops events in the same scenario instead of any errors from web-socket.

Copy link
Collaborator

@luckyj5 luckyj5 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@kashyap-splunk kashyap-splunk merged commit e631a08 into develop Sep 22, 2021
luckyj5 added a commit that referenced this pull request Oct 14, 2021
* update docs (#276)

* Update Go version and migrate to Go modules (#287)

* Adding go.mod and go.sum to migrate to Go modules from Glide

* Removed old Vendor folder after migrating to Go modules

* Adding new Vendor folder after migrating to Go mudules

* Removed Glide files, references in readme and updated CI/CD

* Migrated from bolt to bbolt for cache module to resolve race issue

* Added tags from the envelope to the events

* Updated tile.yml with new config

* Updated README

* Updated migration script to prevent unnecessary migration (#288)

* Updated cache.GetApp logic to retrieve app info from boltdb database as well

* Bugfix/disconnect due to slow consumer event drop (#289)

* dropping events when the queue is full

* Corrected the newly added test case with data race condition

* minor change in logging

* added event drop warning threshold in config

* minor update in the logging

* Updated comments and log info

* Trimmed white space for ApiEndPoint and SplunkHost

* Updated docs with required field & optional value

* Added default value in README

* Updated events doc

* Removed SPLUNK_VERSION config as not needed anymore

* Removed SplunkVersion from eventsink.splunk_test

* Updated README with app level index routing (#292)

* Updated README with app level index routing

* Removed unnecessary changes

* added required config to enable index routing

* updated index routing doc

* added warning in index routing docs

* upgraded module version

* upgraded gogo/protobuf version

* executed go mod tidy

* upgraded indirect dependency

* updated version in a build example in readme

* Corrected index routing docs

* Added more details (#299)

* Updated docs around

- Add Tags: Performance impact of adding tags in events
- How to detect slow downstream related issues and event drops

* added 'drop_warn_threshold' config in tile (#300)

Co-authored-by: Matthew Heidemann <[email protected]>
Co-authored-by: Shubham Jain <[email protected]>
Co-authored-by: harshit-splunk <[email protected]>
Co-authored-by: harshit-splunk <[email protected]>
@kashyap-splunk kashyap-splunk deleted the bugfix/disconnect-due-to-slow-consumer-event-drop branch December 2, 2021 05:40
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.

2 participants