Document bug in bulk batch writer and rename the class to indicate its buggy #288
Conversation
BulkBatchWriter was using Arrays.compare to compare Accumulo rows which is incorrect because it compares bytes as signed integers. Need to compare the bytes as unsigned integers. Updated the code to correctly compare. This bug would not cause data to go to the wrong tablet it could only cause needless creation of many files for a single tablet.
|
This bug has actually been really useful for testing as it creates bulk imports w/ lots of files for a few tablets. |
But you're still planning on merging it? (Note: there's a conflict in this PR now, because it touched some of the same files as #289, which was merged) |
Thought about making the current behavior configurable OR just leaving the current behavior and adding a comment about how its incorrect and was left as is because its useful for testing. Would not want someone to copy this broken code and use it elsewhere. |
|
I think the rename is sufficient to avoid the risk of somebody using it as an example. If you rename that and push it to the relevant branch(es), is there still something to keep from this PR or should it be closed? |
The changes in this PR no longer fix the bug they only rename the class to FlakyBulkBatchWriter and document the bug in the class. I will update the title of the PR |
keith-turner
changed the title
BulkBatchWriter was using Arrays.compare to compare Accumulo rows which is incorrect because it compares bytes as signed integers. Need to compare the bytes as unsigned integers. Updated the code to correctly compare. This bug would not cause data to go to the wrong tablet it could only cause needless creation of many files for a single tablet.