Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork1.4k
-
The CCJSqlParser.interrupted field should use AtomicBoolean |
BetaWas this translation helpful?Give feedback.
All reactions
👍 1
Replies: 5 comments
-
It is cross-thread parser.interrupted =true; It is possible for ! interrupted to be inlined into a register, resulting in interrupted always being false |
BetaWas this translation helpful?Give feedback.
All reactions
-
It is also unwise to use an ExecutorService to implement timeouts, forcing the parsing work to be limited by the number of threads in the thread pool This is an ideal way of implementation, only need to use a global thread ScheduledThreadPoolExecutor whether to mark a timeout, which will help the parse SQL is limited by the ExecutorService number of threads publicstaticScheduledThreadPoolExecutorscheduledThreadPoolExecutor =newScheduledThreadPoolExecutor(1);privatestaticfinalVarHandleIVH;static {try {IVH =MethodHandles.lookup().findVarHandle(CCJSqlParser.class,"interrupted",boolean.class); }catch (NoSuchFieldException |IllegalAccessExceptione) {thrownewRuntimeException(e); } }publicstaticStatementparseWithTimeOut(CCJSqlParserparser)throwsParseException {ScheduledFuture<?>schedule =scheduledThreadPoolExecutor.schedule( () ->{//todo or use AtomicBoolean.setOpaque or use volatile But there is an additional overhead//todo It's not actually valid Because CCJSqlParser gets interrupted as a normal variableIVH.setOpaque(parser,true) },4000,TimeUnit.MILLISECONDS );try {returnparser.Statement(); }finally {schedule.cancel(false); } } |
BetaWas this translation helpful?Give feedback.
All reactions
-
Or just add CCJSqlParser! interrupted is replaced with a check to see if the current time is greater than the timeout, which allows us to disconnect from the thread altogether |
BetaWas this translation helpful?Give feedback.
All reactions
-
Greetings! Thank you very much for you input which I appreciate a lot. Although your comments are a bit scarce to help my understanding.
|
BetaWas this translation helpful?Give feedback.
All reactions
-
Thank you for your reply. I'm sorry that I don't know English and thus I'm using translation software. As a result, what you see might not be very understandable. VarHandle and AtomicBoolean setOpaque/getOpaque were introduced in JDK 9. For versions prior to JDK 9, using volatile is also acceptable, but it may incur unnecessary runtime overhead. Here, only the compile-time barrier with Opaque semantics is needed to ensure that the variable is not inlined and optimized by the compiler to be stored in a register. It is unreasonable to use ExecutorService to implement timeout at present. For example, in the case of web application, the caller is Tomcat with over 100 threads. When handling requests, if a thread pool with less than 100 threads is used to implement timeout processing for jsqlparser, then jsqlparser will become a bottleneck of the system. The concurrency quantity is limited by jsqlparse The issue can be resolved by using a ScheduledThreadPoolExecutor with only one thread. By parsing the SQL statements directly within the user's calling thread and determining whether there is a timeout in the schedule thread, this approach will not be constrained by the number of threads in the passed-in thread pool. |
BetaWas this translation helpful?Give feedback.
All reactions
This discussion was converted from issue #2221 on April 08, 2025 11:26.