FLUME-3205:Remove unnecessary 'synchronized' in ResettableFileInputStream #192
Conversation
KafkaChannelTest had quite a few test methods so during the build sometimes it caused a timeout. Refactoring it to smaller test classes decrease the chance of timeouts. This closes apache#183 Reviewers: Miklos Csanady, Ferenc Szabo (Viktor Somogyi via Ferenc Szabo)
Set surefire version to the latest and configure it to rerun failed tests. This way the flaky tests will not break the build as often. This closes apache#182 Reviewers: Ferenc Szabo (Miklos Csanady via Ferenc Szabo)
Current derby version contains security vulnerabilities. This update will upgrade to a later version. This closes apache#184 Reviewers: Ferenc Szabo (Miklos Csanady via Ferenc Szabo)
…ctly This closes apache#188 Reviewers: Tristan Stevens, Miklos Csanady (Ferenc Szabo via Denes Arvay)
This change upgrades the netty library to 3.10.6 and also adds netty-all 4.1.17 This closes apache#155 Reviewers: Miklos Csanady (Ferenc Szabo via Denes Arvay)
|
@harishreedharan |
|
Hi @guangxian , Thank you for the PR! I've taken a look at the change and it seems to be a nice catch! Even though there might be no concurrent access to the method in the Flume codebase, there might be access "from the outside". Thus, it might be a good idea to make this change in a backwards compatible way. (Ie. extract the "contents" of the method and leave a synchronized read that calls the non-synchronized, new one or something like that.) Please, let me know what you think. Thank you, Donat |
|
@bessbd Thank you for review. I add a |
|
Can one of the admins verify this patch? |
I found no concurrent access to ResettableFileInputStream object in flume code.
Remove 'synchronized' will improve performance of SpoolDirectorySource.
Tested with a file of 200MB，with 'synchronized' it took 9 seconds to read the file.
Without 'synchronized' it took 7 seconds.