Skip to content

listener: fix inplace update with conn balancer#12748

Merged
mattklein123 merged 6 commits intoenvoyproxy:masterfrom
lambdai:listenerbalance
Sep 14, 2020
Merged

listener: fix inplace update with conn balancer#12748
mattklein123 merged 6 commits intoenvoyproxy:masterfrom
lambdai:listenerbalance

Conversation

@lambdai
Copy link
Contributor

@lambdai lambdai commented Aug 20, 2020

Commit Message:
By sharing the connection balancer among listener configs.
Signed-off-by: Yuchen Dai silentdai@gmail.com

Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
[Optional Runtime guard:]
Fixes #12686
[Optional Deprecated:]

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
@mattklein123 mattklein123 self-assigned this Aug 20, 2020
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for the quick fix. A small test comment. Thank you!

/wait

Comment on lines +269 to +272
bootstrap.mutable_static_resources()
->mutable_listeners(0)
->mutable_connection_balance_config()
->mutable_exact_balance();
Copy link
Member

Choose a reason for hiding this comment

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

Can you make this a setup param and test this both ways (default off is fine just want to have at least 1 test where we have the other config.)

@stale
Copy link

stale bot commented Aug 29, 2020

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 Aug 29, 2020
@stale
Copy link

stale bot commented Sep 6, 2020

This pull request has been automatically closed because it has not had activity in the last 14 days. 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 closed this Sep 6, 2020
@wozz
Copy link
Contributor

wozz commented Sep 13, 2020

was this fixed by another PR? or can we reopen this?

@mattklein123
Copy link
Member

@lambdai can you finish this fix? Otherwise I can do it this week.

@mattklein123 mattklein123 reopened this Sep 13, 2020
@stale stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Sep 13, 2020
@lambdai
Copy link
Contributor Author

lambdai commented Sep 13, 2020

Sorry… updating this pr.

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks LGTM with small comments.

/wait

see a change in behavior.
* logging: add fine-grain logging for file level log control with logger management at administration interface. It can be enabled by option `--enable-fine-grain-logging`.
* logging: change default log format to `"[%Y-%m-%d %T.%e][%t][%l][%n] [%g:%#] %v"` and default value of :option:`--log-format-prefix-with-location` to `0`.
* listener: fixed crash at listener inplace update when connetion load balancer is set.
Copy link
Member

Choose a reason for hiding this comment

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

Can you move this to the bug fix section?

Network::Address::InstanceConstSharedPtr address_;
};
bool use_default_balancer_{false};
}; // namespace
Copy link
Member

Choose a reason for hiding this comment

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

?

name_(name), listener_filters_timeout_(listener_filters_timeout),
continue_on_listener_filters_timeout_(continue_on_listener_filters_timeout),
connection_balancer_(std::make_unique<Network::NopConnectionBalancerImpl>()),

Copy link
Member

Choose a reason for hiding this comment

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

nit: delete newline

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks!

@mattklein123 mattklein123 merged commit 308f2a7 into envoyproxy:master Sep 14, 2020
listener_filters_timeout_(
PROTOBUF_GET_MS_OR_DEFAULT(config, listener_filters_timeout, 15000)),
continue_on_listener_filters_timeout_(config.continue_on_listener_filters_timeout()),
connection_balancer_(origin.connection_balancer_),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

mark

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Listener Crash on Start after enabling exact_balance

3 participants