haridsv commented on PR #8057: URL: https://github.com/apache/hbase/pull/8057#issuecomment-4269666431
I did review the changes and raised the PR after making some tweaks, but of course I am not an expert in HBase code base so relied on the PR reviews to identify anything that I missed. > And for load balancer, changing the stop time calculation changes the behavior, we should discuss first before changing the behavior. Yes, except this load balancer fix, all other fixes are purely in the test itself or in the test framework, so IMHO there is non immediate danger to prod code. How about reverting the load balancer change alone and raising a separate PR so that there can be a discussion around it? Alternatively, if you have any review feedback, I can raise a new PR to address it. > we should have common solution for all tests, not patching a single test I purposefully limited the scope of tmp directory fix to only those that are failing, but it is easy enough change the default and make it applicable to all tests, please let me know if you think this is correct and I can raise a new PR. > We need evidence, not only the conclusion from AI. The conclusion was based on the actual logs from both failed and successful runs and I can attach the full test logs and AI chat history, if one wants to double check. > you should split this PR to several small ones since they are different problems I can definitely ensure of this for future fixes, but considering the work already done (including back porting), IMHO, the right thing to do is to review the PR and see if any of those changes need to be reverted rather than a full revert. Just to add a little more context, I started with a couple of fixes, but when the fix PR itself started hitting new flapper, I kept diagnosing and adding fixes to the same PR until I got a green build. > And if you use AI coding tool to provide a big PR without knowing all the details, at least you should provide the information about the tool and the model you use, and also the prompt you used to generate the PR. Sure, I used Claude Code with the default Sonnet 4.5 model. I used the following format as prompt: `You need to diagnose the HBase test XYZ that is flapping. It had multiple failures and errors in the PR validation build, but none when I ran locally. The source code is at @../../src/apache/hbase/ and the logs for good run is at @good-run and bad run is at @bad-run. Can you compare the logs and try to understand why the run failed and look at the code for a potential fix?` -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
