newServer setting maxCheckFailures makes no sense #4198

Merged
merged 2 commits into from Aug 1, 2016

Projects

None yet

2 participants

@stutiredboy
Contributor

when dnsdist add newServer with maxCheckFailures parameter, such as maxCheckFailures=3, if the downstream server seems to be down, then the dss->currentCheckFailures will increase 1.

but, if dss->currentCheckFailures not reach the maxCheckFailures value, at this time, the downstream server seems to be up, the dss->currentCheckFailures was not reset to 0.

so when dss->currentCheckFailures reach maxCheckFailures, dnsdist will show downstream down, the change to up in the next second.

dnsdist[24734]: Marking downstream 10.63.49.12:53 as 'down'
dnsdist[24734]: Marking downstream 10.63.49.12:53 as 'up'

In my opinion, discontinuous failure retry makes no sense, so I post this pull request.

Thanks.

@rgacogne rgacogne added the dnsdist label Jul 18, 2016
@rgacogne
Member

Hi,

This change makes sense, thank you! Could you just fix the indentation? I know it's not easy as we still have a mix of tabs and spaces in some places.

@rgacogne rgacogne commented on an outdated diff Jul 18, 2016
pdns/dnsdist.cc
@@ -1254,7 +1254,12 @@ void* healthChecksThread()
for(auto& dss : g_dstates.getCopy()) { // this points to the actual shared_ptrs!
if(dss->availability==DownstreamState::Availability::Auto) {
bool newState=upCheck(*dss);
- if (!newState && dss->upStatus) {
+ if (newState) {
+ if (dss->currentCheckFailures != 0) {
@rgacogne
rgacogne Jul 18, 2016 Member

You could perhaps skip that test, but that doesn't matter much.

@stutiredboy
Contributor

rager that. I use space as the indent.

@rgacogne
Member

LGTM

@rgacogne rgacogne merged commit 3075d37 into PowerDNS:master Aug 1, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment