-
Notifications
You must be signed in to change notification settings - Fork 4k
xds: implement server feature fail_on_data_errors #12544
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
xds: implement server feature fail_on_data_errors #12544
Conversation
584e614 to
342c6e1
Compare
kannanjgithub
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review comments.
| "Resource " + resource + " deleted from server"); | ||
| onAmbientError(deletionStatus, processingTracker); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the changes here in the onAbsent method in this PR addresses that.
When fail_on_data_errors is false: The code enters the if (xdsDataErrorHandlingEnabled && !failOnDataErrors) block. This calls onAmbientError() and then immediately returns. This preserves the cached data and treats the deletion as a non-fatal ambient error, as you suggested.
When fail_on_data_errors is true: The if condition is not met, so the code falls through to call onResourceDoesNotExist(). This method then drops the cached resource by setting data = null and calls onResourceChanged() with a NOT_FOUND status to propagate the fatal error.
| // 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just import BootstrapperImpl and directly reference BootstrapperImpl.xdsDataErrorHandlingEnabled in the if condition.
Implements server feature
fail_on_data_errors, per gRFC A88: https://github.com/grpc/proposal/blob/master/A88-xds-data-error-handling.md