Skip to content

Proof of Concept: Introduce ServerFactoryContext#8454

Closed
lambdai wants to merge 11 commits intoenvoyproxy:masterfrom
lambdai:provider
Closed

Proof of Concept: Introduce ServerFactoryContext#8454
lambdai wants to merge 11 commits intoenvoyproxy:masterfrom
lambdai:provider

Conversation

@lambdai
Copy link
Copy Markdown
Contributor

@lambdai lambdai commented Oct 2, 2019

Description:
Introduce ServerFactoryContext which is an implementation of CommonFactoryContext. Comparing to ListenerFactoryContext, ServerFactoryContext has a wider lifetime.
In this PR I also prove ServerFactoryContext would allow multiple listeners share a RdsConfigProvider and save huge of memory and CPU
[x] Compile envoy-static with ServerFactoryContext: RdsConfigProvider, PerFilterConfig, Route, VhdsSubscription should use ServerFactoryContext
[ ] Safely implement ServerFactoryCxtUtil::generateServerFactoryContext
[ ] Server::Instance should implement ServerFactoryContext
[ ] RdsConfigProvider should own RdsSubscription
[ ] RdsConfigProvider should be able to notify multiple listeners and listeners could wait on RdsProvider in ready state
[ ] tons of broken tests due to signature change

Risk Level:
Testing:
Docs Changes:
Release Notes:
Fixes #8303

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
@lambdai
Copy link
Copy Markdown
Contributor Author

lambdai commented Oct 4, 2019

@lizan Please review #8485 instead.

This PR will be updated to describe the full picture. It will be growing bigger and not suitable for review.

@stale
Copy link
Copy Markdown

stale bot commented Oct 11, 2019

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@stale stale bot added the stale stalebot believes this issue/PR has not been touched recently label Oct 11, 2019
@lambdai
Copy link
Copy Markdown
Contributor Author

lambdai commented Oct 12, 2019

Split into #8485 and #8523

@lambdai lambdai closed this Oct 12, 2019
htuch pushed a commit that referenced this pull request Oct 18, 2019
Introduce ServerFactoryContext which is an implementation of CommonFactoryContext. Comparing to ListenerFactoryContext, ServerFactoryContext has a wider lifetime.
This is the prerequisite to share rds configuration among listeners.

Also experimenting move messageValidateVisitor out of CommonFactoryContext. The benefit is that now Server::Instance perfectly implements CommonFactoryContext.

ServerFactoryContext doesn't implement the function. The test shows it is never invoked.
Will put messageValidateVisitor into FactoryContext: this piece of work will be done in follow up PR

This PR is extracted from #8454

Fixes #8303

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale stalebot believes this issue/PR has not been touched recently

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rds: make RouteConfigProvider shared among ListenerImpls, or move factory_context_ outside of Listener.

2 participants