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

Increment records/failed counter for failed writes #445

Merged
merged 6 commits into from
Aug 12, 2022

Conversation

adutra
Copy link
Contributor

@adutra adutra commented Aug 10, 2022

Fixes #444.

@adutra adutra added this to the 1.10.0 milestone Aug 10, 2022
@adutra adutra requested a review from weideng1 August 10, 2022 08:52
@@ -234,6 +237,7 @@ public boolean execute() {
statements
.transform(this::executeStatements)
.transform(queryWarningsHandler)
.transform(failedWritesMonitor)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the root cause of the bug: the monitor that increments the records/failed metric wasn't being invoked here, and so failed writes weren't being accounted for.

@@ -733,7 +738,7 @@ public Publisher<Publisher<Record>> read(
assertStatus(status, STATUS_COMPLETED_WITH_ERRORS);
assertThat(logs.getAllMessagesAsString())
.contains("completed with 11000 errors")
.contains("Records: total: 100,000, successful: 90,000, failed: 10,000")
.contains("Records: total: 100,000, successful: 89,000, failed: 11,000")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This bug report unveiled a flaw in this existing assertion.

@weideng1
Copy link
Collaborator

I've now tested the new binary built from this branch, and can see console reporter correctly showing the number of failed operations. LGTM

@adutra adutra merged commit 71bf849 into datastax:1.x Aug 12, 2022
@adutra adutra deleted the console-reporter-bug branch August 12, 2022 10:14
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.

Display bug in console metrics reporter for the number of failed operations
2 participants