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

Reduce Ingester Allocations #694

Merged
merged 31 commits into from
May 12, 2021
Merged

Conversation

joe-elliott
Copy link
Member

@joe-elliott joe-elliott commented May 12, 2021

What this PR does:
Adds two new fields to tempopb.PushBytesRequest "traces" and "ids" and deprecated the "requests" field. traces are pre-marshalled byte slices containing trace which the ingester can now take directly to the wal without additional marshalling. Also added a TraceBytes object that the ingesters now write to the wal and eventually to the blocks.

Other changes:

  • Many test improvements/additions
  • Improve records check in append_block Find to prevent unnecessary lookups
  • PushBytes supports both the old and new methods of pushing to the ingesters to recud
  • Propagate context through instance.FindTraceByID()
  • Added/extended functionality in the pkg/model package to support the new data encoding.
  • Adjusted PreallocRequest to handle the new byte slices

Breaking Change!
Distributors will immediately attempt to start pushing to the ingesters with the new proto which will cause the new ingesters to silently ignore the new fields. To prevent any dropped spans roll out ingesters first and then distributors once all ingesters have upgraded.

Which issue(s) this PR fixes:
Fixes #658

Performance Before Changes
cpu usage seconds:
image

go gc duration seconds:
image

Performance After Changes
cpu usage seconds:
image

go gc duration seconds:
image

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
@mdisibio
Copy link
Contributor

Ran locally and stability and performance improvements all check out.

@joe-elliott joe-elliott merged commit 8d628d7 into grafana:main May 12, 2021
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.

Performance Improvement: Don't marshal to proto before the WAL
2 participants