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

Getdata use pointslicepool and getdata return to pool #1924

Merged

Conversation

Dieterbe
Copy link
Contributor

@Dieterbe Dieterbe commented Oct 15, 2020

PR on top of #1921 - review that first please

I will rebase this soon to make it less confusing.
This PR does two main things:

  • on the query node if errors happen during getTargets() (whether remote or local or both), clean up (add to pool) any already created series, instead of discarding them
  • on the read node in the /getdata path (that receives queries from query nodes for data) return slices to the pool, always (upon errors or in the no error case, after writing the response) . fix pointSlicePool issues #962 issue 1 - the suspicion is that this should significantly reduce memory usage on read nodes, at least in certain cases.

I have not yet been able to demonstrate memory savings.
Note that snapshots functionality is not working right now, but i have 2 screenshots of read nodes subjected to the same workload. memory is equivalent, but on the "after", allocation rate is significantly reduced. also notice more slices going into the pool, and all gets out of the pool succeeding.

PRE

pre

POST

post

this should help reduce memory usage in query nodes when they decode
/getdata requests coming back
1) change getTargetsLocal to, in event of an error, still collect
   and return models.Series to its caller (it only has two callers:
   getTargets and getData)
2) clarify that getTargetsRemote does the same (this was already the
   case).  Note its only caller is getTargets.

Update getTargets and getData to, in event of an error,
collect intermediate slices and return them to the pool.
@Dieterbe Dieterbe force-pushed the getdata-use-pointslicepool-and-getdata-return-to-pool branch from daa8272 to 80b1b5e Compare October 15, 2020 21:35
@Dieterbe
Copy link
Contributor Author

Dieterbe commented Oct 15, 2020

OK, found something fairly obvious.
the memory amounts are so similar because the "min amount of memory used" is about the same for them (pool cleared)
. but both will always let their heap double due to the GOGC variable. it's just that in the 2nd case that takes more time.
So we can interpret as with this code applied, we can reduce GOGC and end up with a lower heap with only the same cost (frequency of GC's) as before. i will do that experiment next (edit: confirmed, can reduce memory by reducing GOGC and still having similar amount of GC runs as before)

PRE

pre

POST

post

@Dieterbe Dieterbe added this to the sprint-18 milestone Oct 16, 2020
@Dieterbe Dieterbe mentioned this pull request Oct 26, 2020
Copy link
Collaborator

@shanson7 shanson7 left a comment

Choose a reason for hiding this comment

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

LGTM

@Dieterbe Dieterbe merged commit dba010c into master Oct 26, 2020
@Dieterbe Dieterbe deleted the getdata-use-pointslicepool-and-getdata-return-to-pool branch October 26, 2020 14:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pointSlicePool issues
2 participants