[C++][Python] Converter::Extend gets stuck in infinite loop causing OOM if values don't fit in single chunk

[asfimport]

Apologies if this is a duplicate, I haven't found anything related

When creating an arrow table via the python api, the following code runs out of memory after using all the available resources on a box with 512GB of ram. This happens with pyarrow 4.0.0 and 4.0.1. However when running the same code with pyarrow 3.0.0, the memory usage only reaches 5GB (which seems like the appropriate ballpark for the table size).
The code generates a table with a single string column with 1m rows, each string being 3000 characters long.

Not sure whether the issue is python related or not, I haven't tried replicating it from the C++ api.

 

import os, string
import numpy as np
import pyarrow as pa

print(pa.__version__)
np.random.seed(42)

alphabet = list(string.ascii_uppercase)

_col = []
for _n in range(1000):
  k = ''.join(np.random.choice(alphabet, 3000))
  _col += [k] * 1000

table = pa.Table.from_pydict({'col': _col})

Reporter: Laurent Mazare
Assignee: David Li / @lidavidm

Related issues:

• [Python] Pandas to_feather no longer works - runs out of memory (is duplicated by)
• [Python] Processes killed and semaphore objects leaked when reading pandas data (is duplicated by)
• [Python] pyarrow.array memory leak on large string arrays (is duplicated by)

PRs and other links:

• GitHub Pull Request #10470
• GitHub Pull Request #10556

_{Note: This issue was originally created as ARROW-12983. Please see the migration documentation for further details.}

[asfimport]

David Li / @lidavidm:
I can confirm this and provide I think an explanation.

When we convert an array, we append values until we get an out-of-capacity error. Then we finish the current chunk and start a new one, recursively. However, we mistakenly don't account for the values already converted! Hence, if the values don't fit in one chunk, we get stuck in an infinite loop of converting the same values over and over, creating an infinitely growing list.

Here's the code in question: https://github.com/apache/arrow/blob/maint-4.0.x/cpp/src/arrow/util/converter.h#L305-L318

[asfimport]

Weston Pace / @westonpace:
I think it's slightly different.  The converter's Extend method might better be named AddRange.  It looks like...

1. Reserve space for all values

2. Go through values and append them

   In the past there was a python converter and a chunker (which was not a converter).  With 245564c the chunker became itself, a converter.

    

   The chunker's append logic is

3. Try and append to child

4. If child hit capacity error then create a chunk with the current values, reset child, continue

    

   This append logic works.  The chunker's extend (add range) logic is...

    

5. Try and extend child

6. If child hit capacity error then create a chunk with current values, reset child, continue

   This extend logic doesn't work.  That is because the child fails the first step of extend (Reserve space for all values) and never actually adds any items.  So then the chunk that is created is an empty chunk.  Repeat until there are enough empty chunks to run out of memory.

[asfimport]

Weston Pace / @westonpace:
Perhaps the child needs an ExtendAsMuchAsPossible method which would require a ReserveAsMuchAsPossible.

[asfimport]

David Li / @lidavidm:
The Reserve method is another possible bug that needs to be worked around but in this case it's not relevant: the binary builder's reserve is only a guess (because it doesn't know the string lengths) so the allocation error only happens during the actual append. (I confirmed this by adding some logging to the allocator - you can watch it reallocate the data array until it hits the max length, then it fails.) So items are actually appended here.

The case you point out should be fixed too, though.

[asfimport]

Krisztian Szucs / @kszucs:
Issue resolved by pull request 10556
#10556