Visitar URL original
xds: support load reporting all clusters option and fix actual report interval measurement by voidzcy · Pull Request #7209 · grpc/grpc-java · GitHub
Skip to content

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

Merged
voidzcy merged 3 commits into
grpc:masterfrom
voidzcy:impl/support_lrs_send_all_clusters_and_fix_actual_load_report_interval
Jul 22, 2020
Merged

voidzcy merged 3 commits into
grpc:masterfrom
voidzcy:impl/support_lrs_send_all_clusters_and_fix_actual_load_report_interval

Conversation

@voidzcy

@voidzcy voidzcy commented Jul 14, 2020 •

Copy link
Copy Markdown
Contributor

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.

voidzcy added 2 commits July 14, 2020 12:36
…nterval measurement for each individual cluster:eds_service.
@voidzcy
voidzcy requested a review from dapengzhang0 July 16, 2020 00:26
@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
Copy Markdown
Contributor

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
Copy Markdown
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.

2 participants