Re: [PR] HBASE-30073 Test fixes for some flappers and a reproducible error [hbase]
haridsv commented on PR #8057: URL: https://github.com/apache/hbase/pull/8057#issuecomment-4297259416 @Apache9 Apologies for the delayed response. I see you've already taken care of the revert — thank you! I'll go ahead and raise the new PRs now. -- 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]
Re: [PR] HBASE-30073 Test fixes for some flappers and a reproducible error [hbase]
Apache9 commented on PR #8057: URL: https://github.com/apache/hbase/pull/8057#issuecomment-4272414477 Reverting a PR is very common if we have some mistakes in the PR, we can apply it later after fixing the mistakes. At least we should not increase the test timeout limit, and also the load balancer change. And on the tmp directory fix, could you please paste the log sections which shows problems for sharing tmp directory? Or you can paste the AI chat log and the full log file it mentioned so we can review it. > 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. I'd say this is common in HBase, that's why we have a flaky dashboard to show flaky tests. Starting a mini cluster in test is heavy and easy to have races when running on a slow machine... On the AI tool, Sonnet 4.5 is not powerful enough, Opus 4.6 will be bettter. And for debugging flaky tests, I recommend use Codex 5.4 xhigh since HBase is a project live for more than 15 years, we have lots of strange code in our code base, you need to think more and more before actually doing things... -- 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]
Re: [PR] HBASE-30073 Test fixes for some flappers and a reproducible error [hbase]
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]
Re: [PR] HBASE-30073 Test fixes for some flappers and a reproducible error [hbase]
Apache9 commented on PR #8057: URL: https://github.com/apache/hbase/pull/8057#issuecomment-4264693259 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. -- 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]
Re: [PR] HBASE-30073 Test fixes for some flappers and a reproducible error [hbase]
Apache9 commented on PR #8057: URL: https://github.com/apache/hbase/pull/8057#issuecomment-4264674068 Please revert this PR because it is not qualified enough. Wo do not prevent using AI coding tool but at least you should review the code by yourself and be resposible for the code. For example, increasing the timeout value is not the correct way here, you should make the test smaller instead of increasing the timeout. And for load balancer, changing the stop time calculation changes the behavior, we should discuss first before changing the behavior. And on the directory sharing, the method is only called when creating hadoop.log.dir and hadoop.tmp.dir directories, can you confirm that the failures on CI are caused by sharing these two directories? We need evidence, not only the conclusion from AI. And flaky rerun has been a problem for a long time, I've also seen the reruns all failed because of mini cluster already started. But we should have common solution for all tests, not patching a single test. And no matter with or without AI, you should split this PR to several small ones since they are different problems. -- 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]
Re: [PR] HBASE-30073 Test fixes for some flappers and a reproducible error [hbase]
liuxiaocs7 commented on PR #8057: URL: https://github.com/apache/hbase/pull/8057#issuecomment-4244263528 Thanks @haridsv and @charlesconnell, great job! -- 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]
Re: [PR] HBASE-30073 Test fixes for some flappers and a reproducible error [hbase]
charlesconnell merged PR #8057: URL: https://github.com/apache/hbase/pull/8057 -- 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]
