[jira] [Commented] (CALCITE-5009) Transparent JDBC connection re-creation may lead to data loss

2022-04-18 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-5009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17523544#comment-17523544
 ] 

Julian Hyde commented on CALCITE-5009:
--

Fixed in 
[9ddb9376|https://github.com/apache/calcite-avatica/commit/9ddb9376299d990a9bb2c766f08c4c44ba95bfe6].

> Transparent JDBC connection re-creation may lead to data loss
> -
>
> Key: CALCITE-5009
> URL: https://issues.apache.org/jira/browse/CALCITE-5009
> Project: Calcite
>  Issue Type: Bug
>  Components: avatica
>Reporter: Istvan Toth
>Assignee: Istvan Toth
>Priority: Major
> Fix For: avatica-1.21.0
>
>  Time Spent: 50m
>  Remaining Estimate: 0h
>
> Currently, if the server-side JDBC connection goes away because it is expored 
> from the server-side connection cache we attempt to transparently create a 
> new "real" JDBC connection, and continue using that instead of the original 
> connection
> [https://github.com/apache/calcite-avatica/blob/fbdcc62745a0e8920db759fb6bdce564d854e407/core/src/main/java/org/apache/calcite/avatica/AvaticaConnection.java#L796]
> This is fine for most read-only connections, but it can break transaction 
> semantics, which is captured in the "real" connection object.
> {noformat}
> conn.setAutocommit(false)
> stmt = conn.createStatement()
> execute(insert A)
> //Connection lost and object recreated which now proxies a new "real" 
> connection
> execute(insert B)
> conn.commit()
> //We have lost "insert A"{noformat}
> I'm not sure if we synchronize autocommit state of the new connection to the 
> lost one or not, but it's bad either way.
>  
> We should either completely drop this feature, add some logic that avoids it 
> if there is an open transaction and/or only allow it for connections that 
> have the readOnly flag set.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Commented] (CALCITE-5009) Transparent JDBC connection re-creation may lead to data loss

2022-04-01 Thread Josh Elser (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-5009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17516071#comment-17516071
 ] 

Josh Elser commented on CALCITE-5009:
-

Of course! I was the one who wrote this apparently half-baked idea :). Happy to 
review.

> Transparent JDBC connection re-creation may lead to data loss
> -
>
> Key: CALCITE-5009
> URL: https://issues.apache.org/jira/browse/CALCITE-5009
> Project: Calcite
>  Issue Type: Bug
>  Components: avatica
>Reporter: Istvan Toth
>Assignee: Istvan Toth
>Priority: Major
>  Time Spent: 40m
>  Remaining Estimate: 0h
>
> Currently, if the server-side JDBC connection goes away because it is expored 
> from the server-side connection cache we attempt to transparently create a 
> new "real" JDBC connection, and continue using that instead of the original 
> connection
> [https://github.com/apache/calcite-avatica/blob/fbdcc62745a0e8920db759fb6bdce564d854e407/core/src/main/java/org/apache/calcite/avatica/AvaticaConnection.java#L796]
> This is fine for most read-only connections, but it can break transaction 
> semantics, which is captured in the "real" connection object.
> {noformat}
> conn.setAutocommit(false)
> stmt = conn.createStatement()
> execute(insert A)
> //Connection lost and object recreated which now proxies a new "real" 
> connection
> execute(insert B)
> conn.commit()
> //We have lost "insert A"{noformat}
> I'm not sure if we synchronize autocommit state of the new connection to the 
> lost one or not, but it's bad either way.
>  
> We should either completely drop this feature, add some logic that avoids it 
> if there is an open transaction and/or only allow it for connections that 
> have the readOnly flag set.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Commented] (CALCITE-5009) Transparent JDBC connection re-creation may lead to data loss

2022-03-31 Thread Istvan Toth (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-5009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17515699#comment-17515699
 ] 

Istvan Toth commented on CALCITE-5009:
--

Looking at the code some more, we're only retrying if we get a 

NoSuchConnectionException exception from the server side.

The text is:
"Connection not found: invalid id, closed, or expired: "

Looking at the code it seems that the connection object is indeed only removed 
on close or when the cache decides to remove it (capcity, timeout...), but it 
does not get removed on other errors.

This does not affect the patch, but I need to update the description.

> Transparent JDBC connection re-creation may lead to data loss
> -
>
> Key: CALCITE-5009
> URL: https://issues.apache.org/jira/browse/CALCITE-5009
> Project: Calcite
>  Issue Type: Bug
>  Components: avatica
>Reporter: Istvan Toth
>Assignee: Istvan Toth
>Priority: Major
>  Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> Currently, if the server-side JDBC connection goes away for any reason
>  * Avatica connection cache expiry
>  * LB/HA Failover
>  * Some problem with the "real" connection
> we attempt to create a new "real" JDBC connection, and continue using that 
> instead of the original connection
> [https://github.com/apache/calcite-avatica/blob/fbdcc62745a0e8920db759fb6bdce564d854e407/core/src/main/java/org/apache/calcite/avatica/AvaticaConnection.java#L796]
> This is fine for most read-only connections, but it can break transaction 
> semantics, which is captured in the "real" connection object.
> {noformat}
> conn.setAutocommit(false)
> stmt = conn.createStatement()
> execute(insert A)
> //Connection lost and object recreated which now proxies a new "real" 
> connection
> execute(insert B)
> conn.commit()
> //We have lost "insert A"{noformat}
> I'm not sure if we synchronize autocommit state of the new connection to the 
> lost one or not, but it's bad either way.
>  
> We should either completely drop this feature, add some logic that avoids it 
> if there is an open transaction and/or only allow it for connections that 
> have the readOnly flag set.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Commented] (CALCITE-5009) Transparent JDBC connection re-creation may lead to data loss

2022-03-31 Thread Istvan Toth (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-5009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17515178#comment-17515178
 ] 

Istvan Toth commented on CALCITE-5009:
--

Can I ask for a review [~elserj]  ?

> Transparent JDBC connection re-creation may lead to data loss
> -
>
> Key: CALCITE-5009
> URL: https://issues.apache.org/jira/browse/CALCITE-5009
> Project: Calcite
>  Issue Type: Bug
>  Components: avatica
>Reporter: Istvan Toth
>Assignee: Istvan Toth
>Priority: Major
>  Time Spent: 20m
>  Remaining Estimate: 0h
>
> Currently, if the server-side JDBC connection goes away for any reason
>  * Avatica connection cache expiry
>  * LB/HA Failover
>  * Some problem with the "real" connection
> we attempt to create a new "real" JDBC connection, and continue using that 
> instead of the original connection
> [https://github.com/apache/calcite-avatica/blob/fbdcc62745a0e8920db759fb6bdce564d854e407/core/src/main/java/org/apache/calcite/avatica/AvaticaConnection.java#L796]
> This is fine for most read-only connections, but it can break transaction 
> semantics, which is captured in the "real" connection object.
> {noformat}
> conn.setAutocommit(false)
> stmt = conn.createStatement()
> execute(insert A)
> //Connection lost and object recreated which now proxies a new "real" 
> connection
> execute(insert B)
> conn.commit()
> //We have lost "insert A"{noformat}
> I'm not sure if we synchronize autocommit state of the new connection to the 
> lost one or not, but it's bad either way.
>  
> We should either completely drop this feature, add some logic that avoids it 
> if there is an open transaction and/or only allow it for connections that 
> have the readOnly flag set.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Commented] (CALCITE-5009) Transparent JDBC connection re-creation may lead to data loss

2022-03-29 Thread Istvan Toth (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-5009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17513997#comment-17513997
 ] 

Istvan Toth commented on CALCITE-5009:
--

The new parameter will need to be added on website as well.

 

> Transparent JDBC connection re-creation may lead to data loss
> -
>
> Key: CALCITE-5009
> URL: https://issues.apache.org/jira/browse/CALCITE-5009
> Project: Calcite
>  Issue Type: Bug
>  Components: avatica
>Reporter: Istvan Toth
>Assignee: Istvan Toth
>Priority: Major
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> Currently, if the server-side JDBC connection goes away for any reason
>  * Avatica connection cache expiry
>  * LB/HA Failover
>  * Some problem with the "real" connection
> we attempt to create a new "real" JDBC connection, and continue using that 
> instead of the original connection
> [https://github.com/apache/calcite-avatica/blob/fbdcc62745a0e8920db759fb6bdce564d854e407/core/src/main/java/org/apache/calcite/avatica/AvaticaConnection.java#L796]
> This is fine for most read-only connections, but it can break transaction 
> semantics, which is captured in the "real" connection object.
> {noformat}
> conn.setAutocommit(false)
> stmt = conn.createStatement()
> execute(insert A)
> //Connection lost and object recreated which now proxies a new "real" 
> connection
> execute(insert B)
> conn.commit()
> //We have lost "insert A"{noformat}
> I'm not sure if we synchronize autocommit state of the new connection to the 
> lost one or not, but it's bad either way.
>  
> We should either completely drop this feature, add some logic that avoids it 
> if there is an open transaction and/or only allow it for connections that 
> have the readOnly flag set.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Commented] (CALCITE-5009) Transparent JDBC connection re-creation may lead to data loss

2022-03-24 Thread Istvan Toth (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-5009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17511719#comment-17511719
 ] 

Istvan Toth commented on CALCITE-5009:
--

I've been mulling over this.

In the strictest sense, transparent reconnecting always breaks JDBC semantics, 
as we have no idea of how the user changed the state of the connection, nor is 
it possible for us to know that.

For example, even for the simplest non-transactional select statement, the user 
may have set a locale on the connections via a "SET locale=x" SQL command, and 
the new connection would have the default locale instead, delivering unexpected 
results.

> Transparent JDBC connection re-creation may lead to data loss
> -
>
> Key: CALCITE-5009
> URL: https://issues.apache.org/jira/browse/CALCITE-5009
> Project: Calcite
>  Issue Type: Bug
>  Components: avatica
>Reporter: Istvan Toth
>Assignee: Istvan Toth
>Priority: Major
>
> Currently, if the server-side JDBC connection goes away for any reason
>  * Avatica connection cache expiry
>  * LB/HA Failover
>  * Some problem with the "real" connection
> we attempt to create a new "real" JDBC connection, and continue using that 
> instead of the original connection
> [https://github.com/apache/calcite-avatica/blob/fbdcc62745a0e8920db759fb6bdce564d854e407/core/src/main/java/org/apache/calcite/avatica/AvaticaConnection.java#L796]
> This is fine for most read-only connections, but it can break transaction 
> semantics, which is captured in the "real" connection object.
> {noformat}
> conn.setAutocommit(false)
> stmt = conn.createStatement()
> execute(insert A)
> //Connection lost and object recreated which now proxies a new "real" 
> connection
> execute(insert B)
> conn.commit()
> //We have lost "insert A"{noformat}
> I'm not sure if we synchronize autocommit state of the new connection to the 
> lost one or not, but it's bad either way.
>  
> We should either completely drop this feature, add some logic that avoids it 
> if there is an open transaction and/or only allow it for connections that 
> have the readOnly flag set.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)