Bigtable `PartialRowsData` is not an iterable.

[tswast]

It appears to me that if I want to scan through all rows of a Bigtable, Table.read_rows() is the function to use. This returns a PartialRowsData.

Why is PartialRowsData not an iterable? I'd like to consume all the rows for a "hello bigtable" sample by iterating through them. Something like

print('Scan for all greetings:')
for row in table.read_rows():
    print(row.cells[COLUMN_FAMILY_NAME][COLUMN_NAME.encode('UTF-8')][0].decode('UTF-8'))

Instead, it appears that I need to use a while loop and watch a dictionary that gets populated. I think this means that even if I don't need to store all the rows in memory at once, they still do. That's both inefficient and not following Python idioms (of generators / iterables).

[dhermes]

Taking a peek at PartialRowsData.consume_next(), it's because of the nature of the Bigtable API. The actual stream returned from the API may have partial results for a row, hence we need to build it up to get a complete answer. I agree this is memory hungry and this was something I worried about (tried to track down old discussions, can't find any right now).

You don't need to use a while loop, you can just use

partial_rows = table.read_rows()
partial_rows.consume_all()

Maybe we should make this clearer in the Data API doc?

[tswast]

Yeah, it could could definitely be clearer in the docs. I'll see if I can improve them at all as I work on the bigtable "hello world".

I can imagine consume_all() being super memory hungry, so I would still like an easy way to consume in a specified batch size.

@dhermes What do you think about adding an iterrows() function/generator to PartialRowsData that yields a (key, row) tuple and deletes that row from rows after calling consume_next()?

[dhermes]

I'm 100% for making this actually stream results instead of storing them in a dictionary. I just don't know what the implications are of giving users partially built results due to chunking. Could you sit down with someone on the eng team for a chat / sanity check?

[tswast]

I've thought about this some more (and read the source code). I'm starting to think the current implementation is the only way it can work, at least if we want to provide full rows in the client.

Maybe we could return a partial row and then later another partial row with the same key? I'm wondering if that behavior would be more intuitive than filling the client's memory. Probably not, actually.

[dhermes]

Thanks for giving it some thought!

[flunderpero]

AFAICS all the other APIs (Go and Java) provide a true iterator-interface. And it is really needed in Python, too. Otherwise a lot of use-case are simply impossible to implement.

It should be as simple as: Whenever the state switches to NEW_ROW we know that we have read a complete row before that and we can yield that (assuming the iterator is written as a generator). This seems to be what the Go-code is doing.

[dhermes]

Thanks for bringing this to our attention @flunderpero. Maybe I misunderstand CellChunk.row_status. When you say NEW_ROW, you are referring to commit_row?

ISTM that you are describing a temporary dict that we pop (and yield) from every time commit_row is encountered? Essentially the same underlying data structure that currently exists, except we don't allow it to grow without bound, since we pop a given row once the API tells us to move on.

[garye]

This is something the Bigtable team would like to see in the API. I'm happy to sign up to do the work, which I believe could be making PartialRowsData iterable with the method @dhermes describes above.

Happybase does something like this now. https://github.com/GoogleCloudPlatform/google-cloud-python-happybase/blob/master/src/google/cloud/happybase/table.py#L394

[garye]

The motivation here is that, while one row can be a maximum of 256MiB, reading the entire result of a read_rows call with consume_all could quickly lead to memory issues on the client.

[dhermes]

Works for me @garye.

@lukesneeringer WDYT (re: prioritization)?