8000 Solve excessive memory consumption in multi-iterators - BTS-1088 (#17… · arangodb/arangodb@d0c4840 · GitHub
[go: up one dir, main page]
Skip to content

Commit d0c4840

Browse files
jsteemannneunhoefKVS85
authored
Solve excessive memory consumption in multi-iterators - BTS-1088 (#17419)
* Free sub-iterators as soon as they are no longer needed. * CHANGELOG. * fix potential nullptr dereference Co-authored-by: Max Neunhoeffer <max@arangodb.com> Co-authored-by: Vadim <vadim@arangodb.com>
1 parent 03436c1 commit d0c4840

File tree

4 files changed

+28
-4
lines changed

4 files changed

+28
-4
lines changed

‎CHANGELOG

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
v3.8.9 (XXXX-XX-XX)
22
-------------------
33

4+
* Solve a case of excessive memory consumption in certain AQL queries with IN
5+
filters with very long lists. Free sub-iterators as soon as they are
6+
exhausted.
7+
8+
* BTS-1070: Fixed query explain not dealing with an aggregate function without
9+
arguments and the WINDOW node not being defined as an Ast node type name.
10+
411
* Fixed error handling of coordinator commit of streaming transactions.
512
This bug could mean that a streaming transaction is reported to be
613
successfully committed when in reality it isn't.
@@ -9,9 +16,6 @@ v3.8.9 (XXXX-XX-XX)
916
v3.8.8 (2022-10-17)
1017
-------------------
1118

12-
* BTS-1070: Fixed query explain not dealing with an aggregate function without
13-
arguments and the WINDOW node not being defined as an Ast node type name.
14-
1519
* Fixed BTS-852 (user's saved queries used to disappear after updating user
1620
profile).
1721

‎arangod/Indexes/IndexIterator.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ IndexIterator::IndexIterator(LogicalCollection* collection, transaction::Methods
3434
: _collection(collection),
3535
_trx(trx),
3636
_hasMore(true),
37+
_resetInternals(false),
3738
_readOwnWrites(readOwnWrites) {
3839
TRI_ASSERT(_collection != nullptr);
3940
TRI_ASSERT(_trx != nullptr);
@@ -205,6 +206,8 @@ bool MultiIndexIterator::nextImpl(LocalDocumentIdCallback const& callback, size_
205206
return false;
206207
}
207208
if (!_current->nextImpl(cb, limit)) {
209+
// Destroy iterator no longer used:
210+
_current->reset();
208211
_currentIdx++;
209212
if (_currentIdx >= _iterators.size()) {
210213
_current = nullptr;
@@ -233,6 +236,8 @@ bool MultiIndexIterator::nextDocumentImpl(DocumentCallback const& callback, size
233236
return false;
234237
}
235238
if (!_current->nextDocumentImpl(cb, limit)) {
239+
// Destroy iterator no longer used:
240+
_current->reset();
236241
_currentIdx++;
237242
if (_currentIdx >= _iterators.size()) {
238243
_current = nullptr;
@@ -267,6 +272,8 @@ bool MultiIndexIterator::nextCoveringImpl(DocumentCallback const& callback, size
267272
return false;
268273
}
269274
if (!_current->nextCoveringImpl(cb, limit)) {
275+
// Destroy iterator no longer used:
276+
_current->reset();
270277
_currentIdx++;
271278
if (_currentIdx >= _iterators.size()) {
272279
_current = nullptr;

‎arangod/Indexes/IndexIterator.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,9 @@ class IndexIterator {
162162
/// provides the "nextCovering" method as a performance optimization
163163
/// The default index has no covering method information
164164
virtual bool hasCovering() const { return false; }
165-
165+
166+
void setResetInternals() noexcept { _resetInternals = true; }
167+
166168
protected:
167169
ReadOwnWrites canReadOwnWrites() const noexcept { return _readOwnWrites; }
168170

@@ -185,6 +187,7 @@ class IndexIterator {
185187
LogicalCollection* _collection;
186188
transaction::Methods* _trx;
187189
bool _hasMore;
190+
bool _resetInternals;
188191

189192
private:
190193
ReadOwnWrites const _readOwnWrites;

‎arangod/RocksDBEngine/RocksDBVPackIndex.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,9 @@ class RocksDBVPackIndexIterator final : public IndexIterator {
301301
void resetImpl() override {
302302
TRI_ASSERT(_trx->state()->isRunning());
303303
_mustSeek = true;
304+
if (_resetInternals) {
305+
_iterator.reset();
306+
}
304307
}
305308

306309
/// @brief we provide a method to provide the index attribute values
@@ -344,6 +347,7 @@ class RocksDBVPackIndexIterator final : public IndexIterator {
344347
}
345348
options.readOwnWrites = canReadOwnWrites() == ReadOwnWrites::yes;
346349
});
350+
TRI_ASSERT(_mustSeek);
347351
}
348352

349353
TRI_ASSERT(_iterator != nullptr);
@@ -1487,10 +1491,16 @@ std::unique_ptr<IndexIterator> RocksDBVPackIndex::iteratorForCondition(
14871491
transaction::BuilderLeaser expandedSearchValues(trx);
14881492
expandInSearchValues(searchValues.slice(), *(expandedSearchValues.get()));
14891493
VPackSlice expandedSlice = expandedSearchValues->slice();
1494+
VPackValueLength numIterators = expandedSlice.length();
1495+
14901496
std::vector<std::unique_ptr<IndexIterator>> iterators;
1497+
iterators.reserve(numIterators);
14911498

14921499
for (VPackSlice val : VPackArrayIterator(expandedSlice)) {
14931500
iterators.push_back(lookup(trx, val, !opts.ascending, readOwnWrites));
1501+
if (numIterators >= 50) {
1502+
iterators.back()->setResetInternals();
1503+
}
14941504
}
14951505

14961506
if (!opts.ascending) {

0 commit comments

Comments
 (0)
0