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

feat: Use IEnumerable in WriteApi to eliminate unnecessary memory all… #615

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

birdalicious
Copy link

@birdalicious birdalicious commented Feb 15, 2024

Proposed Changes

Change WriteApi to use IEnumerable for collection arguments instead of List.
Change WriteRecords(List<string> records) to WriteRecords(IEnumerable<string> records)

Calling ToList on a collection will incur a memory allocation as the data is copied to the new list. Making the argument IEnumerable means ToList doesn't need to be called and so no extra memory allocation.
Internally this will eliminate the unnecessary memory allocation in WriteRecords(string[] record) call to ToList, and client code also won't have to make the allocation if their data isn't already in a List## Checklist

Checklist

  • CHANGELOG.md updated
  • Rebased/mergeable
  • A test has been added if appropriate
  • dotnet test completes successfully
  • Commit messages are conventional
  • Sign CLA (if not already signed)

@birdalicious
Copy link
Author

Sorry for the long delay in updating this PR. I've attempted to run the test suit but most the tests fail on
The request was canceled due to the configured HttpClient.Timeout of 10 seconds elapsing. or similar error messages. I've checked the master branch also does this for me. Is there something I'm missing, I've ran the influx-restart.sh script and have the containers running.

@bednar
Copy link
Contributor

bednar commented Feb 19, 2024

Can you check that the InfluxDB 2 is running on the port 9999 by: curl -i -X GET 'http://localhost:9999/health' ?

@birdalicious
Copy link
Author

It is running and the server replies, all three are up and replying
image

Copy link
Contributor

@bednar bednar left a comment

Choose a reason for hiding this comment

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

@birdalicious birdalicious force-pushed the ienumerable-writeapi branch 3 times, most recently from 74b885d to f03a38f Compare August 9, 2024 10:10
@birdalicious birdalicious force-pushed the ienumerable-writeapi branch from f03a38f to 5c457c6 Compare August 9, 2024 10:23
@birdalicious birdalicious requested a review from bednar August 11, 2024 09:06
@birdalicious
Copy link
Author

@bednar fixed the code formatting

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.

2 participants