Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Fix Version/s: 0.7 beta 1
    • Component/s: Core
    • Labels:
      None
    1. 1127.txt
      56 kB
      Jonathan Ellis
    2. 1127.txt
      40 kB
      Jonathan Ellis

      Issue Links

        Activity

        Hide
        Jonathan Ellis added a comment -

        committed

        Show
        Jonathan Ellis added a comment - committed
        Hide
        Gary Dusbabek added a comment -

        +1

        Show
        Gary Dusbabek added a comment - +1
        Hide
        Jonathan Ellis added a comment -

        rebased

        Show
        Jonathan Ellis added a comment - rebased
        Hide
        Stu Hood added a comment -

        > I wonder if now would be a good time to do a little more sstable cleanup? Here are some things I came across:
        For what it's worth, the 2nd and 4th issues are handled in 1117.

        Show
        Stu Hood added a comment - > I wonder if now would be a good time to do a little more sstable cleanup? Here are some things I came across: For what it's worth, the 2nd and 4th issues are handled in 1117.
        Hide
        Gary Dusbabek added a comment -

        I'd like to give Stu a chance to explain his motivation for CASSANDRA-1117 before giving a +1 here.

        But since I did a review, I wonder if now would be a good time to do a little more sstable cleanup? Here are some things I came across:

        • IndexSummary should be package protected. If SSTR weren't already so long it could probably be made a private subclass there.
        • SSTR.loadBloomFilter, loadIndexFile and mmap should be private methods.
        • the methods that return SSTableScanners should probably be put into a factory method(s) in SSTableScanner.
        • the multitude of factory methods, and private and package constructors in SSTR make hard to understand. I think there is room for consolidation.
        Show
        Gary Dusbabek added a comment - I'd like to give Stu a chance to explain his motivation for CASSANDRA-1117 before giving a +1 here. But since I did a review, I wonder if now would be a good time to do a little more sstable cleanup? Here are some things I came across: IndexSummary should be package protected. If SSTR weren't already so long it could probably be made a private subclass there. SSTR.loadBloomFilter, loadIndexFile and mmap should be private methods. the methods that return SSTableScanners should probably be put into a factory method(s) in SSTableScanner. the multitude of factory methods, and private and package constructors in SSTR make hard to understand. I think there is room for consolidation.
        Hide
        Gary Dusbabek added a comment -

        I am in the middle of reviewing both of these patches and noticed that they are related and won't mix well.

        Show
        Gary Dusbabek added a comment - I am in the middle of reviewing both of these patches and noticed that they are related and won't mix well.

          People

          • Assignee:
            Jonathan Ellis
            Reporter:
            Jonathan Ellis
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development