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

incremental: support for linear and mux caches #459

Merged
merged 14 commits into from
Aug 16, 2021
Merged

incremental: support for linear and mux caches #459

merged 14 commits into from
Aug 16, 2021

Conversation

s-matyukevich
Copy link
Contributor

No description provided.

@s-matyukevich
Add delta xDS support to Linear and Mux caches
ce80713 
Signed-off-by: Sergey Matyukevich <[email protected]>
@s-matyukevich
Add more tests
e6b4caf 
Signed-off-by: Sergey Matyukevich <[email protected]>
s-matyukevich
s-matyukevich commented Jul 2, 2021
View reviewed changes
pkg/cache/v3/delta.go Outdated Show resolved Hide resolved
alecholmez
alecholmez reviewed Jul 20, 2021
View reviewed changes
pkg/cache/v3/linear.go Outdated Show resolved Hide resolved
@s-matyukevich
merge with main
758c527 
Signed-off-by: Sergey Matyukevich <[email protected]>
@s-matyukevich
Merge branch 'main' of https://github.com/envoyproxy/go-control-plane …
29e88a0 
…into incremental-linear
@s-matyukevich
merge with main
ed17309 
Signed-off-by: Sergey Matyukevich <[email protected]>
@s-matyukevich
Refactor resourceContainer interface
0f16bca 
Signed-off-by: Sergey Matyukevich <[email protected]>
@s-matyukevich
merge with main
c57c226 
Signed-off-by: Sergey Matyukevich <[email protected]>
@s-matyukevich
Copy link
Contributor Author

@alecholmez I did some refactoring in the delta.go file and created 2 separate functions: respondDeltaSnapshot and respondDeltaLinear, while still reusing all code from createDeltaResponse Let me know what do you think, maybe it addresses some of your concerns.

alecholmez
alecholmez reviewed Aug 5, 2021
View reviewed changes
Copy link
Contributor

@alecholmez alecholmez left a comment

Choose a reason for hiding this comment

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

I had a few questions here, if we're moving to fix up the snapshot API and revisit some of the old stuff, what can we do here to require the least amount of boilerplate to be added

pkg/cache/v3/delta.go Outdated Show resolved Hide resolved
pkg/cache/v3/delta.go Outdated Show resolved Hide resolved
pkg/cache/v3/linear.go Outdated Show resolved Hide resolved
s-matyukevich
s-matyukevich commented Aug 12, 2021
View reviewed changes
pkg/cache/v3/linear.go Outdated Show resolved Hide resolved
alecholmez
alecholmez requested changes Aug 12, 2021
View reviewed changes
Copy link
Contributor

@alecholmez alecholmez left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Just a few small things/nits. This is definitely starting to look good, thanks for bearing with me on the review process!

pkg/cache/v3/delta.go Outdated Show resolved Hide resolved
pkg/cache/v3/delta.go Show resolved Hide resolved
pkg/cache/v3/snapshot.go Show resolved Hide resolved
pkg/cache/v3/snapshot.go Outdated Show resolved Hide resolved
pkg/cache/v3/linear.go Show resolved Hide resolved
pkg/cache/v3/linear.go Outdated Show resolved Hide resolved
@s-matyukevich @alecholmez
Update pkg/cache/v3/delta.go
4d5c8f5 
Co-authored-by: Alec Holmes <[email protected]>
Signed-off-by: Sergey Matyukevich <[email protected]>
@s-matyukevich
make format
66990d5 
Signed-off-by: Sergey Matyukevich <[email protected]>
@alecholmez alecholmez changed the title Add delta xDS support to Linear and Mux caches incremental: add delta xDS support to linear and mux caches Aug 13, 2021
@alecholmez alecholmez changed the title incremental: add delta xDS support to linear and mux caches incremental: support for linear and mux caches Aug 13, 2021
alecholmez
alecholmez requested changes Aug 13, 2021
View reviewed changes
Copy link
Contributor

@alecholmez alecholmez left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks! A few more comments. I think we've nailed down the overall design just a few more nits on my part

pkg/cache/v3/delta.go Show resolved Hide resolved
pkg/cache/v3/snapshot.go Outdated Show resolved Hide resolved
pkg/cache/v3/snapshot.go Outdated Show resolved Hide resolved
pkg/cache/v3/snapshot.go Outdated Show resolved Hide resolved
pkg/cache/v3/linear.go Outdated Show resolved Hide resolved
pkg/cache/v3/linear.go Outdated Show resolved Hide resolved
alecholmez
alecholmez requested changes Aug 16, 2021
View reviewed changes
Copy link
Contributor

@alecholmez alecholmez left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Just some log nits, then I think we can merge this

pkg/cache/v3/linear.go Outdated Show resolved Hide resolved
pkg/cache/v3/linear.go Outdated Show resolved Hide resolved
pkg/cache/v3/simple.go Show resolved Hide resolved
@s-matyukevich
all logging tags
2c9fcfb 
Signed-off-by: Sergey Matyukevich <[email protected]>
alecholmez
alecholmez approved these changes Aug 16, 2021
View reviewed changes
Copy link
Contributor

@alecholmez alecholmez left a comment

Choose a reason for hiding this comment

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

This looks good! Thanks for the time and effort on this

@alecholmez alecholmez merged commit 3decb94 into envoyproxy:main Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alecholmez alecholmez alecholmez approved these changes

@snowp snowp Awaiting requested review from snowp

Assignees

@snowp snowp

@alecholmez alecholmez

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@s-matyukevich @alecholmez @snowp