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.

Sign up for GitHub

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

Jump to bottom

(Diff) compare changes for CDS only in XdsClient #2

Draft
wants to merge 31 commits into
base: impl/integrate_lds_rds_eds_in_xds_client
Choose a base branch
from
Draft

(Diff) compare changes for CDS only in XdsClient #2

wants to merge 31 commits into from

Conversation

voidzcy
Copy link
Owner

@voidzcy voidzcy commented Nov 12, 2019

No description provided.

@voidzcy voidzcy force-pushed the impl/integrate_cds_in_xds_client branch from 84ee546 to 31ae809 Compare November 12, 2019 01:54
@voidzcy voidzcy changed the title ( Diff) compare changes for CDS only in XdsClient (Diff) compare changes for CDS only in XdsClient Nov 12, 2019
@voidzcy voidzcy force-pushed the impl/integrate_cds_in_xds_client branch from 31ae809 to e49e860 Compare November 13, 2019 19:20
@voidzcy voidzcy force-pushed the impl/integrate_lds_rds_eds_in_xds_client branch from d872bc2 to dbface1 Compare November 13, 2019 19:48
@voidzcy voidzcy force-pushed the impl/integrate_cds_in_xds_client branch from e49e860 to 9387c9b Compare November 13, 2019 19:57
@voidzcy voidzcy force-pushed the impl/integrate_lds_rds_eds_in_xds_client branch from dbface1 to a8b5ad8 Compare November 13, 2019 23:04
@voidzcy voidzcy force-pushed the impl/integrate_cds_in_xds_client branch 3 times, most recently from bc2f130 to d1af988 Compare November 14, 2019 17:57
@voidzcy voidzcy force-pushed the impl/integrate_lds_rds_eds_in_xds_client branch from bd55cb9 to 08d7b8c Compare November 15, 2019 19:58
@voidzcy voidzcy force-pushed the impl/integrate_cds_in_xds_client branch from d1af988 to cfe5bdd Compare November 15, 2019 20:03
@voidzcy voidzcy force-pushed the impl/integrate_lds_rds_eds_in_xds_client branch from 08d7b8c to 4f555cf Compare November 15, 2019 21:03
@voidzcy voidzcy force-pushed the impl/integrate_cds_in_xds_client branch 2 times, most recently from 4398806 to bc8c4c6 Compare November 15, 2019 21:45
@voidzcy voidzcy force-pushed the impl/integrate_lds_rds_eds_in_xds_client branch from ebe4b3e to 3760201 Compare November 18, 2019 23:57
@voidzcy voidzcy force-pushed the impl/integrate_cds_in_xds_client branch from bc8c4c6 to 50c9bb3 Compare November 19, 2019 00:04
@voidzcy voidzcy force-pushed the impl/integrate_lds_rds_eds_in_xds_client branch from 3760201 to 388e977 Compare November 19, 2019 08:34
@voidzcy voidzcy force-pushed the impl/integrate_lds_rds_eds_in_xds_client branch from 388e977 to 9de8847 Compare November 20, 2019 20:18
@ejona86
stub: Provide bread-crumb to user how to configure StreamObserver bef…
44b26da 
…ore start

This is already in the documentation in CallStreamObserver, but if the user
gets here the error didn't provide an actionable next step. The message now
provides more help of how they should have called the methods instead of
feeling more like a brick wall.
@voidzcy voidzcy force-pushed the impl/integrate_cds_in_xds_client branch from 50c9bb3 to 1264ef7 Compare November 20, 2019 23:43
creamsoup and others added 3 commits November 21, 2019 11:52
@creamsoup
core,xds: add missing JUnit RunWith annotation (grpc#6457)
eaf99cf
@dapengzhang0
remove getLoadStatsStore method from LocalityStore
81efecd 
Currently `LoadStatsStore` is create by `LocalityStore` and `LoadReportClient` retrieves `LoadStatsStore` from `LocalityStore.getLoadStatsStore()`.

But `LocalityStore` is create by EDS policy, whereas `LoadReportClient` and `LoadStatsStore` should be created by CDS (if not EDS-only), before `LocalityStore` is created. If `LoadReportClient` is embedded in `XdsClientImpl`, it need a `LoadStatsStore` which shouldn't be created by `LocalityStore`.

Instead, `LoadStatsStore` should be create before `LocalityStore` is created, and be passed to `LocalityStore`'s constructor. A getter is not needed.
@voidzcy
api, core: make scheduled executor service accessible for NameResolve…
eb21c64 
…r.Args (grpc#6455)

Added new API on NameResolver.Args to access ScheduledExecutorService, which is wrapped transport executor.
@voidzcy voidzcy force-pushed the impl/integrate_lds_rds_eds_in_xds_client branch from 5f83d44 to dea0b46 Compare November 22, 2019 20:40
zhangkun83 and others added 5 commits November 22, 2019 16:13
@zhangkun83
netty: enable io.grpc.netty.useCustomAllocator by default (grpc#6459)
b0e00fd
@voidzcy
xds: integrate EDS protocol into XdsClient (grpc#6370)
0a0d9f6 
This change implements EDS protocol in XdsClient, which requests for endpoint information for a given cluster for gRPC's load balancing usage. Note EDS is a quasi-incremental protocol, which means each response may contain only a subset of present resources. Absence of resources should be revealed by response timeout. Currently, timeout is not implemented, endpoint watchers will wait indefinitely if endpoints for the cluster interested in does not exist.
@voidzcy
Implemented EDS response handler.
531df7e
@voidzcy
Add tests for EDS protocol and endpoint watchers.
d5c67d1
@voidzcy
Fixed typo in comments.
950cdd6
voidzcy added 11 commits November 25, 2019 00:07
@voidzcy
Add tests for EDS protocol and endpoint watchers.
ee4d0f4
@voidzcy
Add tests for EDS protocol and endpoint watchers.
3a98dcb
@voidzcy
Added CDS handling with add/cancel cluster watchers in XdsClient.
5cfbf9e
@voidzcy
Do not forward error to endpoint watchers when the proto message is b…
ccbc1e5 
…roken. We should treat it as if had not received the message.
@voidzcy
CDS response handler should notify cluster watchers with for clusters…
6801aaa 
… not found in CDS response.
@voidzcy
Fixed bug of looking up wrong watcher for CDS response.
a9fba1f
@voidzcy
Make a copy (instead of keep the reference) to latest requested CDS r…
2fc7900 
…esources to be able to ACK with last unsubscribed resource when all resources are unsubscribed.
@voidzcy
Added tests for CDS response handling and add/remove cluster watchers.
617c239
@voidzcy
Complete retry tests for incorperating all xds protocols.
e38400f
@voidzcy
Changed flag name from invalidData to dataInvalid
81f875b
@voidzcy
Run due tasks for scheduled 0-delay retries.
1230951
@voidzcy voidzcy force-pushed the impl/integrate_cds_in_xds_client branch from 1264ef7 to 1230951 Compare November 25, 2019 08:32
@voidzcy voidzcy force-pushed the impl/integrate_cds_in_xds_client branch from 49af78e to d325ccc Compare November 26, 2019 19:22
voidzcy pushed a commit that referenced this pull request Mar 25, 2020
@zhangkun83
core: prevent data race in ManagedChannelOrphanWrapper (grpc#6854)
2e3ad1d 
Data race was detected internally when
ManagedChannelOrphanWrapper.shutdown() was called concurrently:

WARNING: ThreadSanitizer: data race (pid=5009)
  Write of size 8 at 0x7fd2f7f37530 by thread T49:
    #0 java.lang.ref.Reference.clear()V Reference.java:265
    #1 io.grpc.internal.ManagedChannelOrphanWrapper$ManagedChannelReference.clearInternal()V ManagedChannelOrphanWrapper.java:118
    #2 io.grpc.internal.ManagedChannelOrphanWrapper$ManagedChannelReference.clear()V ManagedChannelOrphanWrapper.java:110
    #3 io.grpc.internal.ManagedChannelOrphanWrapper.shutdown()Lio/grpc/ManagedChannel; ManagedChannelOrphanWrapper.java:58
    (stacktrace redacted)

  Previous write of size 8 at 0x7fd2f7f37530 by thread T45 (mutexes: write M267260296638793720, write M267541771615505864, write M267823246592216728, write M267260296898451984, write M267541771875162784, write M267823246851873416):
    #0 java.lang.ref.Reference.clear()V Reference.java:265
    #1 io.grpc.internal.ManagedChannelOrphanWrapper$ManagedChannelReference.clearInternal()V ManagedChannelOrphanWrapper.java:118
    #2 io.grpc.internal.ManagedChannelOrphanWrapper$ManagedChannelReference.clear()V ManagedChannelOrphanWrapper.java:110
    #3 io.grpc.internal.ManagedChannelOrphanWrapper.shutdown()Lio/grpc/ManagedChannel; ManagedChannelOrphanWrapper.java:58
    (stacktrace redacted)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@voidzcy @ejona86 @creamsoup @dapengzhang0 @zhangkun83