Auth: add support for multiple carbon servers #3872

Merged
merged 1 commit into from May 24, 2016

Projects

None yet

3 participants

@pieterlexis
Member

Could use a review

Closes #3782
Closes #2093

@rgacogne rgacogne commented on an outdated diff May 18, 2016
pdns/auth-carbon.cc
- time_t now=time(0);
- string hostname=arg()["carbon-ourname"];
- if(hostname.empty()) {
- char tmp[80];
- memset(tmp, 0, sizeof(tmp));
- gethostname(tmp, sizeof(tmp));
- char *p = strchr(tmp, '.');
- if(p) *p=0;
- hostname=tmp;
- boost::replace_all(hostname, ".", "_");
- }
- for(const string& entry : entries) {
- str<<"pdns."<<hostname<<".auth."<<entry<<' '<<S.read(entry)<<' '<<now<<"\r\n";
+
+ if (msg.empty()) {
+ ostringstream str;
@rgacogne
rgacogne May 18, 2016 Member

This could be done before entering the for (const auto& carbonServer : carbonServers) loop, it might perhaps be easier to read.

@Habbie
Member
Habbie commented May 24, 2016

LGTM, but: when we do this we may want to make 'sleep' smarter, to prevent one slow/down target from increasing the interval for the others. Also, this PR drops the theoretical ability to change carbon servers at run time.

Still, LGTM, assuming this was tested.

@pieterlexis pieterlexis merged commit 3a520e5 into PowerDNS:master May 24, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pieterlexis pieterlexis deleted the pieterlexis:issue-3782-auth-multiple-carbon branch May 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment