Skip to content
This repository has been archived by the owner on Aug 30, 2019. It is now read-only.

config: add "connection_limit" option to yaml config #443

Merged
merged 1 commit into from Jun 25, 2018
Merged

config: add "connection_limit" option to yaml config #443

merged 1 commit into from Jun 25, 2018

Conversation

gbbr
Copy link
Contributor

@gbbr gbbr commented Jun 25, 2018

This adds the connection_limit configuration option to the yaml
configuration apm_config section to match the ini settings.

@gbbr
config: add "connection_limit" option to yaml config
deb0442 
This adds the `connection_limit` configuration option to the yaml
configuration `apm_config` section to match the `ini` settings.
@gbbr gbbr added this to the next milestone Jun 25, 2018
@gbbr gbbr requested a review from palazzem June 25, 2018 12:10
LotharSee
LotharSee approved these changes Jun 25, 2018
View reviewed changes
Copy link

@LotharSee LotharSee left a comment •
edited

Choose a reason for hiding this comment

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

While I have nothing against adding this attribute to allow parity with the .ini, this is a deprecated option. We should prefer WatchdogMaxConns.
However its code hasn't been removed.

Let's add least add a comment that it is undocumented/deprecated? Or maybe remove it fully now?

@gbbr gbbr merged commit a3c4da4 into master Jun 25, 2018
@gbbr
Copy link
Contributor Author

gbbr commented Jun 25, 2018 •
edited

Sorry @LotharSee only read the second part of your comment now. Let's see what @palazzem has to say (since he requested this change). Based on that answer I will make a follow-up PR.

@gbbr gbbr deleted the gbbr/connlimit branch June 25, 2018 13:31
@gbbr
Copy link
Contributor Author

gbbr commented Jun 25, 2018

One thing to note: "watchdog" kills the agent if the connection limit is exceeded, is this really what we want? The "old" ConnectionLimit simply errors. Seems less brutal.

@palazzem
Copy link

@gbbr for now let's keep that change because we can deal with that later (in a minor release since we're deprecating a behavior). For the WatchDog question, we may revisit the approach.

gbbr added a commit that referenced this pull request Jul 12, 2018
@gbbr
config: add "connection_limit" option to yaml config (#443)
2f7fb97 
This adds the `connection_limit` configuration option to the yaml
configuration `apm_config` section to match the `ini` settings.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@palazzem palazzem palazzem approved these changes

@LotharSee LotharSee LotharSee approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
6.3.1
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@gbbr @palazzem @LotharSee