large allocation when calculating schema digest with 5000 stables #6376
Assignees
Milestone
Comments
|
I am closing this - since we fixed a number of issues related to memory / using non continues memory - if this happens again - please report |
avikivity
changed the title
|
This isn't fixed. |
|
This is due to (trigger warning: generated code): struct mutation_partition_view {
utils::input_stream v;
...
auto rows() const {
return seastar::with_serialized_stream(v, [this] (auto& v) -> decltype(deserialize(std::declval<utils::input_stream&>(), boost::type<std::vector<deletable_row_view>>())) {
auto in = v;
ser::skip(in, boost::type<size_type>());
ser::skip(in, boost::type<tombstone_view>());
ser::skip(in, boost::type<row_view>());
ser::skip(in, boost::type<std::vector<range_tombstone_view>>());
return deserialize(in, boost::type<std::vector<deletable_row_view>>());
});
}
}; |
|
Low priority. It's just a warning, triggers on rare occasions, and in extreme scenarios (5000 tables). I don't understand why 5000 tables triggered a warning on a 1MB allocation though. What's the schema of an individual table? Moving to a git-like system where the overall schema refers to hashes of individual tables will improve this, since then we'll only need 5000 rows, not 5000*n rows. /cc @tgrabiec |
|
Refs #939 |
sorry for the delay. here is the keyspace definition: here is the table definition: here are the "extra" definitions: |
So 1MB -> 25,000 rows -> 5 rows per table. Which is reasonable. |
|
@haaawk let's change drivers to use paged queries when querying system_schema. |
|
Note that such queries will soon start to trigger warnings and fail because we now restrict the amount of memory unpaged queries can consume. |
|
Yes. @haaawk ping. |
|
running the same job (5000 tables) with Scylla 4.3 (ami-01cc969208fae7a3a) - seeing AFAICT this could be causing nodes to be overloaded, and then we see and it generates in SCT these alerts: it happens to all nodes. logs can be found here: |
|
Please decode all the addresses, not just one. |
all of them are already decoded in sct-runner logs i can copy from the file and paste here, if you find it easier to access them |
|
Yes please, it's also searchable here. |
it will be a very long one, but find it in the next few comments |
|
|
|
|
|
|
|
|
grep them from the SCT log, and put all of them together in the following file: |
|
I cannot read it, everything is on one huge line. |
|
@avi or |
|
this is happening again, with 5000 tables test: |
|
This last one appears less important, it's in the REST API. |
|
I've been checking the backtraces of these warnings, but I couldn't point out exactly where they are coming from in some cases, so I'll share what I've found, maybe someone will have a better idea. I'll start with the cases that are rather easy to fix. Ss @avikivity has already found, the most common reason is the deserialization into a vector in In the REST API we are replying to the /column_family/ GET request with an unchunked message containing Another case in which the warn occurs is when we're creating a The case that isn't quite clear happens when calling fill_buffer on some mutation_readers. On a deeper level we are calling |
We should change that field to be
I'm suprised this is triggered in |
For now bytes_ostream is better. managed_bytes is still contiguous outside lsa (to be fixed soonish).
Did it in fact trigger? Seems unlikely for schema tables. |
I find it strange too. |
This was referenced Jan 13, 2021
|
I've checked the warnings from older runs of the test and there is no |
|
The cell sizes are determined by the user. We can make the allocations discontiguous by switching to manaaged_bytes, @michoecho made lots of progress in #7820. |
wmitros
added a commit
to wmitros/scylla
that referenced
this issue
Feb 28, 2021
The entire sstable cell value is currently stored in a single temporary_buffer. Cells may be very large, so to avoid large contiguous allocations, the buffer is changed to a fragmented_temporary_buffer. Fixes scylladb#7457 Fixes scylladb#6376 Signed-off-by: Wojciech Mitros <wojciech.mitros@scylladb.com>
wmitros
added a commit
to wmitros/scylla
that referenced
this issue
Mar 31, 2021
The entire sstable cell value is currently stored in a single temporary_buffer. Cells may be very large, so to avoid large contiguous allocations, the buffer is changed to a fragmented_temporary_buffer. Fixes scylladb#7457 Fixes scylladb#6376 Signed-off-by: Wojciech Mitros <wojciech.mitros@scylladb.com>
wmitros
added a commit
to wmitros/scylla
that referenced
this issue
Mar 31, 2021
The entire sstable cell value is currently stored in a single temporary_buffer. Cells may be very large, so to avoid large contiguous allocations, the buffer is changed to a fragmented_temporary_buffer. Fixes scylladb#7457 Fixes scylladb#6376 Signed-off-by: Wojciech Mitros <wojciech.mitros@scylladb.com>
avikivity
added a commit
that referenced
this issue
Apr 22, 2021
…jciech Mitros sstable cells are parsed into temporary_buffers, which causes large contiguous allocations for some cells. This is fixed by storing fragments of the cell value in a fragmented_temporary_buffer instead. To achieve this, this patch also adds new methods to the fragmented_temporary_buffer(size(), ostream& operator<<()) and adds methods to the underlying parser(primitive_consumer) for parsing byte strings into fragmented buffers. Fixes #7457 Fixes #6376 Closes #8182 * github.com:scylladb/scylla: primitive_consumer: keep fragments of parsed buffer in a small_vector sstables: add parsing of cell values into fragmented buffers sstables: add non-contiguous parsing of byte strings to the primitive_consumer utils: add ostream operator<<() for fragmented_temporary_buffer::view compound_type: extend serialize_value for all FragmentedView types
psarna
pushed a commit
that referenced
this issue
Apr 22, 2021
…jciech Mitros sstable cells are parsed into temporary_buffers, which causes large contiguous allocations for some cells. This is fixed by storing fragments of the cell value in a fragmented_temporary_buffer instead. To achieve this, this patch also adds new methods to the fragmented_temporary_buffer(size(), ostream& operator<<()) and adds methods to the underlying parser(primitive_consumer) for parsing byte strings into fragmented buffers. Fixes #7457 Fixes #6376 Closes #8182 * github.com:scylladb/scylla: primitive_consumer: keep fragments of parsed buffer in a small_vector sstables: add parsing of cell values into fragmented buffers sstables: add non-contiguous parsing of byte strings to the primitive_consumer utils: add ostream operator<<() for fragmented_temporary_buffer::view compound_type: extend serialize_value for all FragmentedView types
|
Not a regression, and not a small fix, so not backporting. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Installation details
Scylla version (or git commit hash): 4.0.rc3-0.20200501.eee4c00e29 with build-id 0fe095b80ee54c7f47a688aaac0c6bb835118ddc
Cluster size: started with 1 and then added more 5
OS (RHEL/CentOS/Ubuntu/AWS AMI): ami-0352ad86d128c6419
seen such message on all nodes...
the 1st time it happened when there was one single node (the test creates one single node, then creates 5000 tables, and then add (5) nodes) during compaction:
decoded backtrace:
logs can be found here:
The text was updated successfully, but these errors were encountered: