Skip to content
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

xds: support load reporting all clusters option and fix actual report interval measurement #7209

Conversation

voidzcy
Copy link
Contributor

@voidzcy voidzcy commented Jul 14, 2020

Two changes for LoadReportClient in this PR:

  • Add support for send_all_clusters field in LRS response.

    • When it is set to true, just send load reports for clusters that the client is currently tracking (aka, is sending load to).
  • The actual load report interval (in each ClusterStats message, which contains the stats for each cluster:eds_service) is tracked individually.

    • Example:
      Initially, if the client has load stats recorded for cluster-foo:eds_service-foo and cluster-bar:eds_service-bar while the management server is only asking for load stats for cluster-foo:eds_service-foo to be reported every 10 seconds. The actual LRS requests sent to the management server should contain only one ClusterStats message (for cluster-foo:eds_service-foo) with load report interval being 10 seconds. After a while, the management server asks load stats for cluster-bar:eds_service-bar. Then the first LRS request after that should contain ClusterStats for cluster-bar:eds_service-bar should set its actual load report interval to the whole period from the beginning (Although the definition of "beginning" is unclear here. We use the time when the client starts to know the cluster while Envoy uses epoch. I think it shouldn't matter much for the first report of the cluster:eds_service). The point of this change is the actual load report interval for each cluster should be tracked individually as loads for clusters are not necessarily reported together.

    • For reference, see Enovy's implementation.

…nterval measurement for each individual cluster:eds_service.
@dapengzhang0 dapengzhang0 self-assigned this Jul 17, 2020
private LoadStatsEntity(LoadStatsStore loadStatsStore, Stopwatch stopwatch) {
this.loadStatsStore = loadStatsStore;
this.stopwatch = stopwatch;
stopwatch.reset().start();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to pass a StopwatchSupplier in constructor, and create a new stopwatch here, so that it's clear that the stopwatch's lifecycle is solely managed by the LoadStatsEntity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to take the stopwatch supplier from the LrsClient directly.

@voidzcy voidzcy merged commit 9f49e48 into grpc:master Jul 22, 2020
voidzcy added a commit to voidzcy/grpc-java that referenced this pull request Aug 11, 2020
… interval measurement (grpc#7209)

- Add support for send_all_clusters field in LRS response. When it is set to true, just send load reports for clusters that the client is currently tracking (aka, is sending load to).

- The actual load report interval (in each ClusterStats message, which contains the stats for each cluster:eds_service) should be tracked individually.
voidzcy added a commit to voidzcy/grpc-java that referenced this pull request Aug 11, 2020
… interval measurement (grpc#7209)

- Add support for send_all_clusters field in LRS response. When it is set to true, just send load reports for clusters that the client is currently tracking (aka, is sending load to).

- The actual load report interval (in each ClusterStats message, which contains the stats for each cluster:eds_service) should be tracked individually.
voidzcy added a commit that referenced this pull request Aug 11, 2020
… interval measurement (v1.31.x backport) (#7209) (#7315)

- Add support for send_all_clusters field in LRS response. When it is set to true, just send load reports for clusters that the client is currently tracking (aka, is sending load to).

- The actual load report interval (in each ClusterStats message, which contains the stats for each cluster:eds_service) should be tracked individually.
dfawley pushed a commit to dfawley/grpc-java that referenced this pull request Jan 15, 2021
… interval measurement (grpc#7209)

- Add support for send_all_clusters field in LRS response. When it is set to true, just send load reports for clusters that the client is currently tracking (aka, is sending load to).

- The actual load report interval (in each ClusterStats message, which contains the stats for each cluster:eds_service) should be tracked individually.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants