Details

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

      Description

      Let it be clear however that until CASSANDRA-4415 is resolved, it will put us in a situation where it will be easy to write queries that timeout (and potentially OOM the server). That being said, even with the auto-limit it's not too hard to write queries that timeout if you're not at least a bit careful and so far we've always answer that by saying 'you have to be mindful of how much data your query is asking for'. And while I'm all for adding protection against OOMing the server like suggested by Jonathan on CASSANDRA-4304, I think the arbitrary auto-limit is the worst possible solution to this problem.

      Note that until CASSANDRA-4415 is resolved I wouldn't be totally opposed to force people to provide a LIMIT to select queries if we're really thing it will avoids lots of surprise, though tbh I do think it would be enough to just continue to be vocal about the fact that 'you have to be mindful of how much data your query is asking for' and its follow-up 'you should use an explicit LIMIT if in doubt about how much data will be returned'.

      But I am strongly opposed in keeping the current arbitrary limit because it makes very little sense imo, and the little sense it makes will completely vanish once CASSANDRA-4415 is here, and I don't want to break the API and do a CQL4 to be able to remove that limit later.

      1. 4918.txt
        0.9 kB
        Sylvain Lebresne

        Activity

        Hide
        slebresne Sylvain Lebresne added a comment -

        Trivial patch attached

        Show
        slebresne Sylvain Lebresne added a comment - Trivial patch attached
        Hide
        jbellis Jonathan Ellis added a comment -

        How about we remove this server-side, but add it to cqlsh? (Maybe even cut it down to 1K rows there.) That would address my worries about OOMing the server adequately.

        Show
        jbellis Jonathan Ellis added a comment - How about we remove this server-side, but add it to cqlsh? (Maybe even cut it down to 1K rows there.) That would address my worries about OOMing the server adequately.
        Hide
        jbellis Jonathan Ellis added a comment -

        (that is: if querystring does not contain limit, cqlsh could add on LIMIT 1000 as a default.)

        Show
        jbellis Jonathan Ellis added a comment - (that is: if querystring does not contain limit , cqlsh could add on LIMIT 1000 as a default.)
        Hide
        slebresne Sylvain Lebresne added a comment -

        I'm clearly fine with that, though while the server-side patch is attached, I'd rather let the cqlsh part to someone that knows the cqlsh code better if possible.

        Show
        slebresne Sylvain Lebresne added a comment - I'm clearly fine with that, though while the server-side patch is attached, I'd rather let the cqlsh part to someone that knows the cqlsh code better if possible.
        Hide
        jbellis Jonathan Ellis added a comment -

        +1 then. Created CASSANDRA-4972 for the cqlsh change. (Would like to get that into 1.2.0 but it's not critical.)

        Show
        jbellis Jonathan Ellis added a comment - +1 then. Created CASSANDRA-4972 for the cqlsh change. (Would like to get that into 1.2.0 but it's not critical.)
        Hide
        slebresne Sylvain Lebresne added a comment -

        Alright, committed, thanks

        Show
        slebresne Sylvain Lebresne added a comment - Alright, committed, thanks

          People

          • Assignee:
            slebresne Sylvain Lebresne
            Reporter:
            slebresne Sylvain Lebresne
            Reviewer:
            Jonathan Ellis
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development