xds: implement server feature fail_on_data_errors #12544
Conversation
shivaspeaks
force-pushed
the
server_feature_fail_on_data_errors
branch
from
584e614 to
342c6e1
Compare
| // delete cached data so onError will call onResourceChanged instead of onAmbientError. | ||
| // When xdsDataErrorHandlingEnabled is false, use old behavior (always keep cached data). | ||
| boolean xdsDataErrorHandlingEnabled = | ||
| io.grpc.xds.client.BootstrapperImpl.xdsDataErrorHandlingEnabled; |
There was a problem hiding this comment.
Just import BootstrapperImpl and directly reference BootstrapperImpl.xdsDataErrorHandlingEnabled in the if condition.
kannanjgithub
left a comment
kannanjgithub
left a comment
There was a problem hiding this comment.
Some comments on tests.
| * Tests that a NACKed LDS resource update drops the cached resource when fail_on_data_errors | ||
| * is enabled. | ||
| */ | ||
| @Test |
There was a problem hiding this comment.
We also need to test the behavior with the feature flag enabled but the server feature fail_on_data_error is false. The _subscribedResourceInvalid tests test with both these conditions false, so it should be easy to duplicate one of the tests for feature flag enabled and with the same results.
| */ | ||
| @Test | ||
| public void ldsResourceDeleted_failOnDataErrors_false() { | ||
| BootstrapperImpl.xdsDataErrorHandlingEnabled = true; |
There was a problem hiding this comment.
We should also test the case when this is false but the server feature is present (same results as this test).
| assertThat( | ||
| receivedStatuses.stream().anyMatch( | ||
| status -> status.getCode() == Status.Code.NOT_FOUND | ||
| && status.getDescription().contains("Resource C deleted from server"))) |
There was a problem hiding this comment.
Since this test already tests resource deletions, do we need the next set of tests separately for the same?
There was a problem hiding this comment.
By keeping both, we have simple unit tests for the individual behaviors and a more complex test for their interaction.
2 participants
Implements server feature
fail_on_data_errors, per gRFC A88: https://github.com/grpc/proposal/blob/master/A88-xds-data-error-handling.md