Skip to content

Bigtable PartialRowsData is not an iterable. #1812

Closed

Description

@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).

Activity

  1. dhermes commented on May 23, 2016

    @dhermes
    Contributor

    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?

  2. tswast commented on May 23, 2016

    @tswast
    ContributorAuthor

    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()?

  3. dhermes commented on May 23, 2016

    @dhermes
    Contributor

    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?

  4. added
    type: feature request‘Nice-to-have’ improvement, new feature or different behavior or design.
    on Jun 23, 2016
  5. tswast commented on Jun 23, 2016

    @tswast
    ContributorAuthor

    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.

  6. dhermes commented on Jun 29, 2016

    @dhermes
    Contributor

    Thanks for giving it some thought!

  7. flunderpero commented on Dec 20, 2016

    @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.

  8. dhermes commented on Dec 21, 2016

    @dhermes
    Contributor

    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.

  9. garye commented on Sep 19, 2017

    @garye
    Contributor

    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

  10. garye commented on Sep 19, 2017

    @garye
    Contributor

    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.

  11. dhermes commented on Sep 19, 2017

    @dhermes
    Contributor

    Works for me @garye.

    @lukesneeringer WDYT (re: prioritization)?

  12. zakons commented on Jan 22, 2018

    @zakons
    Contributor

    BigTable: Adding a row generator on a table. #4679 is in final stages of test / review.

  13. chemelnucfin commented on Jan 22, 2018

    @chemelnucfin
    Contributor

    Hello, feature requests will now be tracked in the project Feature Requests. I will close this issue now, please feel free to continue to address any issues/concerns here.

    Closed in #4679

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

api: bigtableIssues related to the Bigtable API.type: feature request‘Nice-to-have’ improvement, new feature or different behavior or design.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions