Skip to content

Notification based Promtail WAL Watcher polling approach#9135

Merged
cstyan merged 20 commits intomainfrom
pablo/notification-based-watcher
May 17, 2023
Merged

Notification based Promtail WAL Watcher polling approach#9135
cstyan merged 20 commits intomainfrom
pablo/notification-based-watcher

Conversation

@thepalbi
Copy link
Copy Markdown
Contributor

@thepalbi thepalbi commented Apr 13, 2023

What this PR does / why we need it:

This PR implements a new mechanism for the wal Watcher in Promtail, to know there are new records to be read. It uses a combination of:

The main idea is that the primary mechanism is a notification channel between the wal.Writer and wal.Watcher. The Watcher subscribes to write events the writer publishes, getting notified if the wal has been written. The same subscriptions design is used for cleanup events.

As a backup, the watcher has a timer that implements an exponential backoff strategy, which is constrained by a minimum and maximum that the user can configure.

Below the cpu difference is shown of running both main and this branch against the same scrape target.

image

The yellow line is the latest main build from where this branch started, and the green line is this branch. Both promtails tailing docker logs, and using the following metrics to get cpu usage from cadvisor:

avg by (name) (rate(container_cpu_usage_seconds_total{job=~".+", instance=~".+", name=~"promtail-wal-test_promtail.+"}[$__rate_interval]))

Which issue(s) this PR fixes:
Part of #8197

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • CHANGELOG.md updated
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/upgrading/_index.md

@thepalbi thepalbi changed the title Pablo/notification based watcher Improve Promtail wal watcher polling approach Apr 13, 2023
@thepalbi thepalbi changed the title Improve Promtail wal watcher polling approach Notification based Promtail WAL Watcher polling approach Apr 13, 2023
@thepalbi thepalbi mentioned this pull request Apr 13, 2023
11 tasks
@thepalbi thepalbi marked this pull request as ready for review April 14, 2023 16:21
@thepalbi thepalbi requested a review from a team as a code owner April 14, 2023 16:21
Copy link
Copy Markdown
Contributor

@cstyan cstyan left a comment

Choose a reason for hiding this comment

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

I think we're missing a way of removing the client/manager from the list of subscribers when it goes away?

@thepalbi
Copy link
Copy Markdown
Contributor Author

I think we're missing a way of removing the client/manager from the list of subscribers when it goes away?

Do we need such subscriber removal action implemented? The only time a subscriber, be it a watcher or a write to, are stopped, is when Promtail is stopped. This could happen when the config is re-loaded, or it's actually stopped. Since when re-loading the whole client.Manager and wal.Writer are re-created from scratch, we don't care about removing the subscribers.

@cstyan
Copy link
Copy Markdown
Contributor

cstyan commented Apr 26, 2023

Since when re-loading the whole client.Manager and wal.Writer are re-created from scratch, we don't care about removing the subscribers.

Got it. I was incorrectly assuming that the Manager struct itself supported reloading, but you're right I just read the code and we create the Manager when reloading the config file.

@thepalbi thepalbi force-pushed the pablo/notification-based-watcher branch from 546cd23 to 901c11e Compare May 9, 2023 13:58
@cstyan cstyan merged commit 306c96c into main May 17, 2023
@cstyan cstyan deleted the pablo/notification-based-watcher branch May 17, 2023 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants