You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Sylvain Lebresne (JIRA)" <ji...@apache.org> on 2012/07/19 11:00:38 UTC

[jira] [Created] (CASSANDRA-4449) Make prepared statement global rather than connection based

Sylvain Lebresne created CASSANDRA-4449:
-------------------------------------------

             Summary: Make prepared statement global rather than connection based
                 Key: CASSANDRA-4449
                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4449
             Project: Cassandra
          Issue Type: Improvement
            Reporter: Sylvain Lebresne


Currently, prepared statements are connection based. A client can only use a prepared statement on the connection it prepared it on, and if you prepare the same prepared statement on multiple connections, we'll keep multiple times the same prepared statement. This is potentially inefficient but can also be fairly painful for client libraries with pool of connections (a.k.a all reasonable client library ever) as this means you need to make sure you prepare statement on every connection of the pool, including the connection that don't exist yet but might be created later.

This ticket suggests making prepared statement global (at least for CQL3), i.e. move them out of ClientState. This will likely reduce the number of stored statement on a given node quite a bit, since it's very likely that all clients to a given node will prepare the same statements (and potentially on all of their connection with the node). And given that prepared statement identifiers are the hashCode() of the string, this should be fairly trivial.

I will note that while I think using a hash of the string as identifier is a very good idea, I don't know if the default java hashCode() is good enough. If that's a concern, maybe we should use a safer (bug longer) hash like md5 or sha1. But we'd better do that now.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (CASSANDRA-4449) Make prepared statement global rather than connection based

Posted by "Sylvain Lebresne (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4449?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13462097#comment-13462097 ] 

Sylvain Lebresne commented on CASSANDRA-4449:
---------------------------------------------

bq. Actually I don't see a reason to use something as heavyweight as MD5.

The advantage of using a hash of the query string as ID is that you only ever store one prepared statement for a given query. Which does save memory in practice because a node will be connected by many clients that will usually all prepare the same set of queries. It also give you some protection against buggy/crappy clients that re-prepared the same query again and again, though that's a more minor point. As for the heavyweightness of MD5, I don't think this matters in the case of prepared statements.


                
> Make prepared statement global rather than connection based
> -----------------------------------------------------------
>
>                 Key: CASSANDRA-4449
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4449
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Sylvain Lebresne
>            Assignee: Sylvain Lebresne
>              Labels: binary_protocol
>             Fix For: 1.2.0 beta 2
>
>         Attachments: 4449.txt, 4449-v2.txt
>
>
> Currently, prepared statements are connection based. A client can only use a prepared statement on the connection it prepared it on, and if you prepare the same prepared statement on multiple connections, we'll keep multiple times the same prepared statement. This is potentially inefficient but can also be fairly painful for client libraries with pool of connections (a.k.a all reasonable client library ever) as this means you need to make sure you prepare statement on every connection of the pool, including the connection that don't exist yet but might be created later.
> This ticket suggests making prepared statement global (at least for CQL3), i.e. move them out of ClientState. This will likely reduce the number of stored statement on a given node quite a bit, since it's very likely that all clients to a given node will prepare the same statements (and potentially on all of their connection with the node). And given that prepared statement identifiers are the hashCode() of the string, this should be fairly trivial.
> I will note that while I think using a hash of the string as identifier is a very good idea, I don't know if the default java hashCode() is good enough. If that's a concern, maybe we should use a safer (bug longer) hash like md5 or sha1. But we'd better do that now.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4449) Make prepared statement global rather than connection based

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4449?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13462102#comment-13462102 ] 

Jonathan Ellis commented on CASSANDRA-4449:
-------------------------------------------

Makes sense.
                
> Make prepared statement global rather than connection based
> -----------------------------------------------------------
>
>                 Key: CASSANDRA-4449
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4449
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Sylvain Lebresne
>            Assignee: Sylvain Lebresne
>              Labels: binary_protocol
>             Fix For: 1.2.0 beta 2
>
>         Attachments: 4449.txt, 4449-v2.txt
>
>
> Currently, prepared statements are connection based. A client can only use a prepared statement on the connection it prepared it on, and if you prepare the same prepared statement on multiple connections, we'll keep multiple times the same prepared statement. This is potentially inefficient but can also be fairly painful for client libraries with pool of connections (a.k.a all reasonable client library ever) as this means you need to make sure you prepare statement on every connection of the pool, including the connection that don't exist yet but might be created later.
> This ticket suggests making prepared statement global (at least for CQL3), i.e. move them out of ClientState. This will likely reduce the number of stored statement on a given node quite a bit, since it's very likely that all clients to a given node will prepare the same statements (and potentially on all of their connection with the node). And given that prepared statement identifiers are the hashCode() of the string, this should be fairly trivial.
> I will note that while I think using a hash of the string as identifier is a very good idea, I don't know if the default java hashCode() is good enough. If that's a concern, maybe we should use a safer (bug longer) hash like md5 or sha1. But we'd better do that now.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (CASSANDRA-4449) Make prepared statement global rather than connection based

Posted by "Sylvain Lebresne (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CASSANDRA-4449?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sylvain Lebresne updated CASSANDRA-4449:
----------------------------------------

    Attachment: 4449-v3.txt

bq. Can we update the Prepared query with ID %s not found IRE to explain that the problem is a client bug?

Very good point. Attaching v3 that not only explain that in the error message but for the binary protocol creates a separate exception for that case. That way, smart client libraries can easily catch that case and act on it (i.e.  prepare the statement).

                
> Make prepared statement global rather than connection based
> -----------------------------------------------------------
>
>                 Key: CASSANDRA-4449
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4449
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Sylvain Lebresne
>            Assignee: Sylvain Lebresne
>              Labels: binary_protocol
>             Fix For: 1.2.0 beta 2
>
>         Attachments: 4449.txt, 4449-v2.txt, 4449-v3.txt
>
>
> Currently, prepared statements are connection based. A client can only use a prepared statement on the connection it prepared it on, and if you prepare the same prepared statement on multiple connections, we'll keep multiple times the same prepared statement. This is potentially inefficient but can also be fairly painful for client libraries with pool of connections (a.k.a all reasonable client library ever) as this means you need to make sure you prepare statement on every connection of the pool, including the connection that don't exist yet but might be created later.
> This ticket suggests making prepared statement global (at least for CQL3), i.e. move them out of ClientState. This will likely reduce the number of stored statement on a given node quite a bit, since it's very likely that all clients to a given node will prepare the same statements (and potentially on all of their connection with the node). And given that prepared statement identifiers are the hashCode() of the string, this should be fairly trivial.
> I will note that while I think using a hash of the string as identifier is a very good idea, I don't know if the default java hashCode() is good enough. If that's a concern, maybe we should use a safer (bug longer) hash like md5 or sha1. But we'd better do that now.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4449) Make prepared statement global rather than connection based

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4449?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13462953#comment-13462953 ] 

Jonathan Ellis commented on CASSANDRA-4449:
-------------------------------------------

+1
                
> Make prepared statement global rather than connection based
> -----------------------------------------------------------
>
>                 Key: CASSANDRA-4449
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4449
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Sylvain Lebresne
>            Assignee: Sylvain Lebresne
>              Labels: binary_protocol
>             Fix For: 1.2.0 beta 2
>
>         Attachments: 4449.txt, 4449-v2.txt, 4449-v3.txt
>
>
> Currently, prepared statements are connection based. A client can only use a prepared statement on the connection it prepared it on, and if you prepare the same prepared statement on multiple connections, we'll keep multiple times the same prepared statement. This is potentially inefficient but can also be fairly painful for client libraries with pool of connections (a.k.a all reasonable client library ever) as this means you need to make sure you prepare statement on every connection of the pool, including the connection that don't exist yet but might be created later.
> This ticket suggests making prepared statement global (at least for CQL3), i.e. move them out of ClientState. This will likely reduce the number of stored statement on a given node quite a bit, since it's very likely that all clients to a given node will prepare the same statements (and potentially on all of their connection with the node). And given that prepared statement identifiers are the hashCode() of the string, this should be fairly trivial.
> I will note that while I think using a hash of the string as identifier is a very good idea, I don't know if the default java hashCode() is good enough. If that's a concern, maybe we should use a safer (bug longer) hash like md5 or sha1. But we'd better do that now.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4449) Make prepared statement global rather than connection based

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4449?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13461934#comment-13461934 ] 

Jonathan Ellis commented on CASSANDRA-4449:
-------------------------------------------

bq. Changed in v2 to CLHM

Can we update the {{Prepared query with ID %s not found}} IRE to explain that the problem is a client bug?  i.e., that either
# it didn't re-prepare across a server restart or
# it prepared too many statements and the LRU ones dropped out

bq. switch the hash used as statement ID to md5

Actually I don't see a reason to use something as heavyweight as MD5.  Why not just use AtomicInteger?  Bonus: this would allow fixing CQL2 as well while maintaining compatibility.
                
> Make prepared statement global rather than connection based
> -----------------------------------------------------------
>
>                 Key: CASSANDRA-4449
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4449
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Sylvain Lebresne
>            Assignee: Sylvain Lebresne
>              Labels: binary_protocol
>             Fix For: 1.2.0 beta 2
>
>         Attachments: 4449.txt, 4449-v2.txt
>
>
> Currently, prepared statements are connection based. A client can only use a prepared statement on the connection it prepared it on, and if you prepare the same prepared statement on multiple connections, we'll keep multiple times the same prepared statement. This is potentially inefficient but can also be fairly painful for client libraries with pool of connections (a.k.a all reasonable client library ever) as this means you need to make sure you prepare statement on every connection of the pool, including the connection that don't exist yet but might be created later.
> This ticket suggests making prepared statement global (at least for CQL3), i.e. move them out of ClientState. This will likely reduce the number of stored statement on a given node quite a bit, since it's very likely that all clients to a given node will prepare the same statements (and potentially on all of their connection with the node). And given that prepared statement identifiers are the hashCode() of the string, this should be fairly trivial.
> I will note that while I think using a hash of the string as identifier is a very good idea, I don't know if the default java hashCode() is good enough. If that's a concern, maybe we should use a safer (bug longer) hash like md5 or sha1. But we'd better do that now.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4449) Make prepared statement global rather than connection based

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4449?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13460599#comment-13460599 ] 

Jonathan Ellis commented on CASSANDRA-4449:
-------------------------------------------

Why not use CLHM and get actual LRU behavior instead of strict remove-oldest behavior?

What does continuing to use clientState for Thrift clients buy us?  If we give unique IDs (and I agree w/ the change to md5) I don't see how our internal implementation details matter.
                
> Make prepared statement global rather than connection based
> -----------------------------------------------------------
>
>                 Key: CASSANDRA-4449
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4449
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Sylvain Lebresne
>            Assignee: Sylvain Lebresne
>              Labels: binary_protocol
>             Fix For: 1.2.0 beta 2
>
>         Attachments: 4449.txt
>
>
> Currently, prepared statements are connection based. A client can only use a prepared statement on the connection it prepared it on, and if you prepare the same prepared statement on multiple connections, we'll keep multiple times the same prepared statement. This is potentially inefficient but can also be fairly painful for client libraries with pool of connections (a.k.a all reasonable client library ever) as this means you need to make sure you prepare statement on every connection of the pool, including the connection that don't exist yet but might be created later.
> This ticket suggests making prepared statement global (at least for CQL3), i.e. move them out of ClientState. This will likely reduce the number of stored statement on a given node quite a bit, since it's very likely that all clients to a given node will prepare the same statements (and potentially on all of their connection with the node). And given that prepared statement identifiers are the hashCode() of the string, this should be fairly trivial.
> I will note that while I think using a hash of the string as identifier is a very good idea, I don't know if the default java hashCode() is good enough. If that's a concern, maybe we should use a safer (bug longer) hash like md5 or sha1. But we'd better do that now.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (CASSANDRA-4449) Make prepared statement global rather than connection based

Posted by "Sylvain Lebresne (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CASSANDRA-4449?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sylvain Lebresne updated CASSANDRA-4449:
----------------------------------------

    Attachment: 4449.txt

Attaching patch that:
# make prepared statement global for the binary protocol. I've kept backward compatibility on the thrift side to be conservative.
# for the binary protocol, switch the hash used as statement ID to md5, because I don't trust String.hashCode() enough (and I don't think there is guarantees that it cannot change between java versions).

I'd *really* like to get that in for 1.2 because I think that having per-connection statement makes it very painful for clients library that want to hide the details of how they perform connection pooling.
                
> Make prepared statement global rather than connection based
> -----------------------------------------------------------
>
>                 Key: CASSANDRA-4449
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4449
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Sylvain Lebresne
>         Attachments: 4449.txt
>
>
> Currently, prepared statements are connection based. A client can only use a prepared statement on the connection it prepared it on, and if you prepare the same prepared statement on multiple connections, we'll keep multiple times the same prepared statement. This is potentially inefficient but can also be fairly painful for client libraries with pool of connections (a.k.a all reasonable client library ever) as this means you need to make sure you prepare statement on every connection of the pool, including the connection that don't exist yet but might be created later.
> This ticket suggests making prepared statement global (at least for CQL3), i.e. move them out of ClientState. This will likely reduce the number of stored statement on a given node quite a bit, since it's very likely that all clients to a given node will prepare the same statements (and potentially on all of their connection with the node). And given that prepared statement identifiers are the hashCode() of the string, this should be fairly trivial.
> I will note that while I think using a hash of the string as identifier is a very good idea, I don't know if the default java hashCode() is good enough. If that's a concern, maybe we should use a safer (bug longer) hash like md5 or sha1. But we'd better do that now.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (CASSANDRA-4449) Make prepared statement global rather than connection based

Posted by "Sylvain Lebresne (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CASSANDRA-4449?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sylvain Lebresne updated CASSANDRA-4449:
----------------------------------------

    Attachment: 4449-v2.txt

bq. Why not use CLHM and get actual LRU behavior instead of strict remove-oldest behavior?

Not only is there no good reason for doing so (I dumbly copy/pasted the existing code) but it's actually wrong since the prepared map is now accessed by multiple threads. Changed in v2 to CLHM.

bq. What does continuing to use clientState for Thrift clients buy us? If we give unique IDs (and I agree w/ the change to md5) I don't see how our internal implementation details matter.

That was mostly laziness on my part to change the minimum relating to thrift.  But I can agree that I doubt anyone rely on the fact that statement are not global, so changed in v2 to make it always global for CQL3.

                
> Make prepared statement global rather than connection based
> -----------------------------------------------------------
>
>                 Key: CASSANDRA-4449
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4449
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Sylvain Lebresne
>            Assignee: Sylvain Lebresne
>              Labels: binary_protocol
>             Fix For: 1.2.0 beta 2
>
>         Attachments: 4449.txt, 4449-v2.txt
>
>
> Currently, prepared statements are connection based. A client can only use a prepared statement on the connection it prepared it on, and if you prepare the same prepared statement on multiple connections, we'll keep multiple times the same prepared statement. This is potentially inefficient but can also be fairly painful for client libraries with pool of connections (a.k.a all reasonable client library ever) as this means you need to make sure you prepare statement on every connection of the pool, including the connection that don't exist yet but might be created later.
> This ticket suggests making prepared statement global (at least for CQL3), i.e. move them out of ClientState. This will likely reduce the number of stored statement on a given node quite a bit, since it's very likely that all clients to a given node will prepare the same statements (and potentially on all of their connection with the node). And given that prepared statement identifiers are the hashCode() of the string, this should be fairly trivial.
> I will note that while I think using a hash of the string as identifier is a very good idea, I don't know if the default java hashCode() is good enough. If that's a concern, maybe we should use a safer (bug longer) hash like md5 or sha1. But we'd better do that now.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira