This repository has been archived by the owner on Jan 28, 2021. It is now read-only.
sql: implement memory management system for caches #802
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
golangcibot
reviewed
Aug 12, 2019
erizocosmico
force-pushed
the
feature/memory
branch
from
95d9aae
to
65596fc
Compare
erizocosmico
force-pushed
the
feature/memory
branch
2 times, most recently
from
3cf6a25
to
f4d9a5b
Compare
|
Fixed all the comments @agarciamontoro |
erizocosmico
force-pushed
the
feature/memory
branch
from
f4d9a5b
to
ef5f971
Compare
|
Fixed. I'm also looking into the problem in the ruby integration test, which is now failing for some reason |
erizocosmico
force-pushed
the
feature/memory
branch
2 times, most recently
from
96c12a3
to
6131062
Compare
|
CI fixed. Cleaned up a few things in the travis file that were not needed anymore |
juanjux
reviewed
Aug 13, 2019
erizocosmico
force-pushed
the
feature/memory
branch
from
6131062
to
a733e4b
Compare
juanjux
approved these changes
Aug 13, 2019
ajnavarro
reviewed
Aug 13, 2019
sql/memory.go
Outdated
| type KeyValueCache interface { | ||
| // Put a new value in the cache. If there is no more memory and the cache is | ||
| // not Freeable it will try to free some memory from other freeable caches. | ||
| // If there's still no more memory available, it will fail and erase all the |
There was a problem hiding this comment.
This shouldn't be the case in all the implementations. Per example, a warmed cache can improve a lot consecutive queries execution over more or less the same data.
There was a problem hiding this comment.
Ok, will remove that from the doc
ajnavarro
reviewed
Aug 13, 2019
|
|
||
| // NewLRUCache returns an empty LRU cache and a function to dispose it when it's | ||
| // no longer needed. | ||
| func (m *MemoryManager) NewLRUCache(size uint) (KeyValueCache, DisposeFunc) { |
There was a problem hiding this comment.
This is intended to be used per query or while the SQL engine is running?
There was a problem hiding this comment.
the same question for all the cache implementations
There was a problem hiding this comment.
While it's running. You can get a cache from any place you have a sql.Context.
This PR implements a memory management system, intended to have control over the allocated memory for caches so that they can be freed at any moment and we can avoid out of memory errors. The main changes are the following: - MemoryManager in the sql package, which is just the component that tracks all caches. Memory of all freeable caches can be freed using the Free method of this component. The only way to instantiate new caches is using the NewXXXCache methods. - Rows, history and LRU cache implementations, accessed using the NewXXXCache methods of MemoryManager. - Reporters, which is a component that reports the maximum amount of memory the program is allowed to use and the currently used memory. This interface is meant for making testing easier. There is a default ProcessMemory reporter that returns the memory used by the process and the maximum memory defined in the `MAX_MEMORY` environment variable. - MemoryManager is passed down to every component through *sql.Context, which meant a little more boilerplate on the server SessionBuilder. - GroupBy, Sort, Distinct and Join now use the provided APIs of memory and cache management for their in-memory computations. Caveats: - We need to think of a good default so that memory usage won't grow forever and crash eventually, which is the behaviour when MAX_MEMORY is 0. Signed-off-by: Miguel Molina <miguel@erizocosmi.co>
erizocosmico
force-pushed
the
feature/memory
branch
from
a733e4b
to
4bc20fc
Compare
ajnavarro
approved these changes
Aug 14, 2019
agarciamontoro
approved these changes
Aug 14, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements a memory management system, intended to have
control over the allocated memory for caches so that they can be
freed at any moment and we can avoid out of memory errors.
The main changes are the following:
tracks all caches. Memory of all freeable caches can be freed
using the Free method of this component. The only way to instantiate
new caches is using the NewXXXCache methods.
NewXXXCache methods of MemoryManager.
of memory the program is allowed to use and the currently used memory.
This interface is meant for making testing easier. There is a default
ProcessMemory reporter that returns the memory used by the process and
the maximum memory defined in the
MAX_MEMORYenvironment variable.which meant a little more boilerplate on the server SessionBuilder.
and cache management for their in-memory computations.
Caveats:
forever and crash eventually, which is the behaviour when MAX_MEMORY is 0.
Signed-off-by: Miguel Molina miguel@erizocosmi.co