jihoonson commented on a change in pull request #11643:
URL: https://github.com/apache/druid/pull/11643#discussion_r700615322



##########
File path: sql/src/main/java/org/apache/druid/sql/SqlLifecycle.java
##########
@@ -80,8 +84,7 @@
  * <li>Logging ({@link #emitLogsAndMetrics(Throwable, String, long)})</li>
  * </ol>
  *
- * <p>Unlike QueryLifecycle, this class is designed to be <b>thread safe</b> 
so that it can be used in multi-threaded
- * scenario (JDBC) without external synchronization.
+ * Every method in this class must be called by the same thread except for 
{@link #cancel()}.

Review comment:
       That javadoc seems no longer valid. `SqlLifecycle` is always used under 
a lock in `DruidStatement` which is the same lock used in 
`DruidStatement.nextFrame()`. I believe locking in `SqlLifecycle` is redundant 
except for sql canceling. Meanwhile, it was necessary to get rid of locking in 
`SqlLifecycle`, especially in `execute()` to be able to cancel the query while 
its execution.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to