ctubbsii commented on pull request #1964: URL: https://github.com/apache/accumulo/pull/1964#issuecomment-798563527
I don't have enough knowledge of the bulk import code to understand exactly what this accomplishes, but it seems to me that this is a slight implementation change intended to leave files around to help troubleshoot, and does not have any associated API changes. Is that correct? Although we're only doing bugfixes on 1.10, I don't necessarily have a problem including a change that helps troubleshooting for further bugfixes, if the change is small and low-risk. However, I did have a few questions about these proposed changes: 1. Does this risk leaving extra files scattered around the filesystem that aren't cleaned up or is this placed in a directory that is already subject to cleanup? 2. Why not use one of the JSON libraries (like Gson) on our class path instead of manually constructing a JSON blob? Is there a performance concern? 3. Does the new behavior of adding a new file risk breaking existing user code around bulk import? 4. How does this change impact the new bulk import code paths and behavior, if it does at all? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
