Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Fix Version/s: 1.1.7, 1.2.0 beta 2
    • Component/s: None
    • Labels:
      None

      Description

      get_paged_slice doesn't reset the start column filter for the second returned row sometimes. So instead of getting a slice:

      row 0: <start_column>...<last_column_in_row>
      row 1: <first column in a row>...<last_column_in_row>
      row 2: <first column in a row>...

      you sometimes get:

      row 0: <start_column>...<last_column_in_row>
      row 1: <start_column>...<last_column_in_row>
      row 2: <first column in a row>...

      1. 4816-3.txt
        8 kB
        Piotr Kołaczkowski
      2. 4816-2.txt
        4 kB
        Sylvain Lebresne

        Activity

        Hide
        Jonathan Ellis added a comment -

        LGTM, committed

        Show
        Jonathan Ellis added a comment - LGTM, committed
        Hide
        Sylvain Lebresne added a comment -

        To be clear, I'm also good on version 3 but I'll let Jonathan review since I've wrote version 2 on which version 3 is based.

        Show
        Sylvain Lebresne added a comment - To be clear, I'm also good on version 3 but I'll let Jonathan review since I've wrote version 2 on which version 3 is based.
        Hide
        Cathy Daw added a comment -

        +1 this version fixes my tests as well.

        Show
        Cathy Daw added a comment - +1 this version fixes my tests as well.
        Hide
        Piotr Kołaczkowski added a comment - - edited

        Attaching 3rd version of the patch. LazyColumnIterator is used both for SSTableScanners and Memtable.
        This version works for me.

        Show
        Piotr Kołaczkowski added a comment - - edited Attaching 3rd version of the patch. LazyColumnIterator is used both for SSTableScanners and Memtable. This version works for me.
        Hide
        Sylvain Lebresne added a comment -

        Ok, second attempt. Looking more closing it does seem that this is a problem with the interaction of the mergeIterator and the SSTableScanner. Basically the mergeIterator always needs to know what his "the next row" (during the reducing phase). If that next row was the one that the reducer ended up returning, we were fine (so with 1 sstable or if all sstables had the same rows, it was ok), but otherwise it might end up using the QueryFilter before it should for our get_paged_slice "hack".

        Anyway, all that the mergeIterator needs during its reduction phase is to know the next key. So attaching a patch that delay the use of the filter until the row data is actually queried.

        Show
        Sylvain Lebresne added a comment - Ok, second attempt. Looking more closing it does seem that this is a problem with the interaction of the mergeIterator and the SSTableScanner. Basically the mergeIterator always needs to know what his "the next row" (during the reducing phase). If that next row was the one that the reducer ended up returning, we were fine (so with 1 sstable or if all sstables had the same rows, it was ok), but otherwise it might end up using the QueryFilter before it should for our get_paged_slice "hack". Anyway, all that the mergeIterator needs during its reduction phase is to know the next key. So attaching a patch that delay the use of the filter until the row data is actually queried.
        Hide
        Sylvain Lebresne added a comment -

        Scratch that patch, brain fart. The AbstractIterator.computeNext() is called no sooner than on hasNext() so we should be good on that front. Still don't know what is the problem here.

        Show
        Sylvain Lebresne added a comment - Scratch that patch, brain fart. The AbstractIterator.computeNext() is called no sooner than on hasNext() so we should be good on that front. Still don't know what is the problem here.
        Hide
        Sylvain Lebresne added a comment -

        My bet would be that this is due to the memtable iterator. The way we implement get_paged_slice, we reset the start of the QueryFilter used after the first row have been read. Which means that for each row, we need to make sure the filter is not used until we've processed the preceding row. This is the case for the sstable iterator, but not for the memtable one. Attaching patch to change that (I haven't tested the patch though).

        Show
        Sylvain Lebresne added a comment - My bet would be that this is due to the memtable iterator. The way we implement get_paged_slice, we reset the start of the QueryFilter used after the first row have been read. Which means that for each row, we need to make sure the filter is not used until we've processed the preceding row. This is the case for the sstable iterator, but not for the memtable one. Attaching patch to change that (I haven't tested the patch though).

          People

          • Assignee:
            Piotr Kołaczkowski
            Reporter:
            Piotr Kołaczkowski
            Reviewer:
            Sylvain Lebresne
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development