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

[8.14] fix: Update FlushBytes parsing/defaults (backport #13576) #13577

Merged
merged 5 commits into from
Jul 5, 2024

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Jul 4, 2024

Motivation/summary

Updates the FlushBytes setting to default to 1 mib and only override to 24kb if the user has explicitly set it to 24kb.

Checklist

How to test these changes

  1. Spin up an APM Server and send data to it (without this fix).
  2. Look at the request length for Elasticsearch requests and verify they're smaller than 50kb and very close to 24kb (26kb)
  3. Now start apm sever with the code in this PR and send data to it
  4. Verify that the flush size is significantly higher than before and if you sent enough data it should be ~1MB.

Related issues

Fixes #13024


This is an automatic backport of pull request #13576 done by [Mergify](https://mergify.com).

@mergify mergify bot added backport conflicts There is a conflict in the backported pull request labels Jul 4, 2024
@mergify mergify bot assigned marclop Jul 4, 2024
Copy link
Contributor Author

mergify bot commented Jul 4, 2024

Cherry-pick of a453a88 has failed:

On branch mergify/bp/8.14/pr-13576
Your branch is up to date with 'origin/8.14'.

You are currently cherry-picking commit a453a8850.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   internal/beater/beater_test.go

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	deleted by us:   changelogs/head.asciidoc
	both modified:   internal/beater/beater.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Copy link
Member

@carsonip carsonip left a comment

Choose a reason for hiding this comment

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

is 8.14 the right version?

edit: ack 8.14.3

@inge4pres
Copy link
Contributor

is 8.14 the right version?

I think so, because this is a backport, and we don't release 8.13 anymore?

@inge4pres inge4pres requested a review from carsonip July 4, 2024 13:42
@marclop marclop removed the conflicts There is a conflict in the backported pull request label Jul 5, 2024
@marclop marclop dismissed carsonip’s stale review July 5, 2024 06:51

Releasing an 8.14.3. So this needs to be merged into 8.14

@marclop marclop force-pushed the mergify/bp/8.14/pr-13576 branch 2 times, most recently from 18d2bf4 to 17fb71e Compare July 5, 2024 07:02
marclop and others added 5 commits July 5, 2024 15:13
Updates the `FlushBytes` setting to default to 1 mib and only override
to 24kb if the user has explicitly set it to 24kb.

Fixes #13024
---------

Signed-off-by: Marc Lopez Rubio <[email protected]>
(cherry picked from commit a453a88)
Signed-off-by: Marc Lopez Rubio <[email protected]>

# Conflicts:
#	changelogs/head.asciidoc
#	internal/beater/beater.go
Signed-off-by: inge4pres <[email protected]>
Signed-off-by: Marc Lopez Rubio <[email protected]>
Signed-off-by: inge4pres <[email protected]>
Signed-off-by: Marc Lopez Rubio <[email protected]>
Signed-off-by: inge4pres <[email protected]>
Signed-off-by: Marc Lopez Rubio <[email protected]>
Signed-off-by: Marc Lopez Rubio <[email protected]>
@marclop marclop force-pushed the mergify/bp/8.14/pr-13576 branch from 17fb71e to 278260c Compare July 5, 2024 07:14
@marclop marclop enabled auto-merge (squash) July 5, 2024 07:19
@marclop marclop merged commit 1b2815f into 8.14 Jul 5, 2024
10 checks passed
@marclop marclop deleted the mergify/bp/8.14/pr-13576 branch July 5, 2024 07:33
@marclop
Copy link
Contributor

marclop commented Jul 5, 2024

Tested this on ESS with a 1GB APM Server and an 8GB Elasticsearch. Documents per bulk request is up to 4-5K and median request length is 800-1MB, so the small bulk_indexer size regression is gone.

image image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants