-
Notifications
You must be signed in to change notification settings - Fork 558
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
fix(exporters/elasticsearch-exporter): avoid OOM when retrying #4787
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Just two small optional suggestions.
@@ -118,7 +125,10 @@ public void bulk(final Map<String, Object> command, final Record<?> record) { | |||
"Failed to serialize bulk request command to JSON", e); | |||
} | |||
|
|||
bulkRequest.add(serializedCommand + "\n" + record.toJson()); | |||
final String jsonCommand = serializedCommand + "\n" + record.toJson(); | |||
if (bulkRequest.isEmpty() || !bulkRequest.get(bulkRequest.size() - 1).equals(jsonCommand)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could add a comment here why we're filtering it out. I think it is not so obvious.
when(recordMock.toJson()).thenReturn("{}"); | ||
client.index(recordMock); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A new-line could improve the reading flow.
when(recordMock.toJson()).thenReturn("{}"); | |
client.index(recordMock); | |
when(recordMock.toJson()).thenReturn("{}"); | |
client.index(recordMock); |
d04ef34
to
9916533
Compare
bors r+ |
Build succeeded |
4789: [BACKPORT] fix(exporters/elasticsearch-exporter): avoid OOM exception when retrying r=saig0 a=MiguelPires ## Description Don't append record again when retrying to export, in order to avoid OOM exception. ## Related issues closes #4668 backport of #4787 # Co-authored-by: Miguel Pires <miguel.pires@camunda.com>
4790: [BACKPORT] fix(exporters/elasticsearch-exporter): avoid OOM exception when retrying r=saig0 a=MiguelPires ## Description Don't append record again when retrying to export, in order to avoid OOM exception. ## Related issues closes #4668 backport of #4787 # Co-authored-by: Miguel Pires <miguel.pires@camunda.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Vinicius Goulart <vinicius.goulart@camunda.com>
Description
Don't append record again when retrying to export, in order to avoid OOM exception.
Related issues
closes #4668
Pull Request Checklist
mvn clean install -DskipTests
locally before committing