-
Notifications
You must be signed in to change notification settings - Fork 404
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
Issue 6924: Failure in system test for io.pravega.test.system.LargeEventTest.largeEventSimpleTest #6955
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Raúl Gracia <raul.gracia@emc.com>
Signed-off-by: Raúl Gracia <raul.gracia@emc.com>
Signed-off-by: Raúl Gracia <raul.gracia@emc.com>
Signed-off-by: Raúl Gracia <raul.gracia@emc.com>
Signed-off-by: Raúl Gracia <raul.gracia@emc.com>
Signed-off-by: Raúl Gracia <raul.gracia@emc.com>
Signed-off-by: Raúl Gracia <raul.gracia@emc.com>
Signed-off-by: Raúl Gracia <raul.gracia@emc.com>
Codecov ReportBase: 86.34% // Head: 86.34% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #6955 +/- ##
=========================================
Coverage 86.34% 86.34%
+ Complexity 15910 15907 -3
=========================================
Files 1027 1027
Lines 59273 59253 -20
Branches 5997 5995 -2
=========================================
- Hits 51180 51163 -17
Misses 4955 4955
+ Partials 3138 3135 -3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
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.
It may be the case that there is some deeper error here. We should keep looking into the issue because it may affect the normal append path as well.
val reply = client.sendRequest(requestId, request); | ||
failFast(futures, created.getSegment()); | ||
futures.add(reply); | ||
// Wait for Appends to complete. Writing in parallel Appends that are expected to be sequential does not help. |
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.
This isn't at all true. We support append pipelining on both the client and the server. This will avoid round trips for each of them and allow them to be written sequentially with the client starting the next write as soon as the previous has finished.
Please restore the failfast/join at the end.
Change log description
Removed pipelining of writes in
LargeEventWriter
as it sometimes leads to block the writer.Purpose of the change
Fixes #6924.
What the code does
Removed pipelining of writes in
LargeEventWriter
as it sometimes leads to block the writer. Now, the writer waits for each 8MB append future to be completed to write the next one.How to verify it
Failing system test consistently passes with this change.