sharmaar12 commented on PR #7149: URL: https://github.com/apache/hbase/pull/7149#issuecomment-3270604017
> I have some suggestions, but LGTM overall. > > I have one thought about the unit tests. I see the `refresh_hfiles` is being performed, but the tests don't switch the cluster in and out of read-only mode when doing the refresh. Should they be doing that or is that more suitable for an integration test? > > PR #7058 has a test case similar to what I'm describing, but it is with `refresh_meta`. Here is the test case I am referring to: https://github.com/apache/hbase/pull/7058/files#diff-17ebd8f29298a3dcce8564212c6712346dc62fd02dac5239ca3f83302eb13439 I have added test variations which just checks for refresh procedure calls executes successfully when readonly is enabled. The case of readonly disabled is covered in other variation so did not repeat it. This test verify refresh_hfiles should succeed on read replica cluster. If we have to check workflow where the data gets manipulated and data consistency gets checked at the end of refresh should be done in an integration test as it involve testing read-only property, dynamic modification of property value and refresh_hfiles together hence, in my opinion better candidate for integration testing. cc: @wchevreuil @anmolnar -- 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]
