Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve InfluxdbWriter performance #5764

Merged
merged 1 commit into from Dec 12, 2017
Merged

Conversation

gunnarbeutner
Copy link
Contributor

This also depends on PR #5759 and #5760.

@gunnarbeutner gunnarbeutner added enhancement New feature or request area/influxdb Metrics to InfluxDB labels Nov 15, 2017
@gunnarbeutner gunnarbeutner added this to the 2.9.0 milestone Nov 15, 2017
}
}

msgbuf << " " << static_cast<unsigned long>(ts);

#ifdef _DEBUG
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be I2_DEBUG.

}

double et = Utility::GetTime();
Log(LogInformation, "InfluxDbWriter")
Copy link
Contributor

Choose a reason for hiding this comment

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

That might be too noisy as information level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, that's not even meant to be there at all, at least not in the PR. 😇

@dnsmichi dnsmichi self-requested a review November 15, 2017 12:55
@dnsmichi
Copy link
Contributor

That's de facto a rewrite of the InfluxDB feature. Must be tested with care before any release.

@gunnarbeutner
Copy link
Contributor Author

Aye, this is definitely a major change.

@dnsmichi dnsmichi added the core/quality Improve code, libraries, algorithms, inline docs label Dec 7, 2017
@dnsmichi
Copy link
Contributor

dnsmichi commented Dec 7, 2017

Backporting will be "funny" with the code quality changes applied in master.

@dnsmichi
Copy link
Contributor

dnsmichi commented Dec 8, 2017

#5759 and #5760 have been merged to master, a test backport to 2.8 went fine. Now for rebasing this PR.

@gunnarbeutner
Copy link
Contributor Author

Backporting as per @lippserd's request. :)

@gunnarbeutner gunnarbeutner self-assigned this Dec 12, 2017
@gunnarbeutner gunnarbeutner modified the milestones: 2.9.0, 2.8.1 Dec 12, 2017
@gunnarbeutner gunnarbeutner merged commit b4e72d5 into master Dec 12, 2017
@gunnarbeutner gunnarbeutner deleted the feature/influxdb-cleanup branch December 12, 2017 09:57
@dnsmichi dnsmichi modified the milestones: 2.8.1, 2.9.0 Jan 16, 2018
@dnsmichi
Copy link
Contributor

We agreed on reverting this from 2.8.x as the InfluxDBWriter performance fixes require more reliable and long-term tests. Such changes will only hit master and 2.9 then.

Required reverts:

#5759
#5760
#5764 [x]

dnsmichi pushed a commit that referenced this pull request Jan 16, 2018
@N-o-X N-o-X modified the milestones: 2.9.0, 2.8.2 Jan 17, 2018
@N-o-X N-o-X added the backported Fix was included in a bugfix release label Jan 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/influxdb Metrics to InfluxDB backported Fix was included in a bugfix release core/quality Improve code, libraries, algorithms, inline docs enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants