[jira] [Commented] (QPID-8534) [Broker-J] Improper interruption of connection processing loop
[ https://issues.apache.org/jira/browse/QPID-8534?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17612107#comment-17612107 ] ASF GitHub Bot commented on QPID-8534: -- mklaca closed pull request #97: QPID-8534: [Broker-J] Improper interruption of connection processing loop URL: https://github.com/apache/qpid-broker-j/pull/97 > [Broker-J] Improper interruption of connection processing loop > -- > > Key: QPID-8534 > URL: https://issues.apache.org/jira/browse/QPID-8534 > Project: Qpid > Issue Type: Improvement > Components: Broker-J >Reporter: Marek Laca >Priority: Minor > Labels: Broker, Connection, Java > > The network connection scheduler (NetworkConnectionScheduler class) contains > a thread pool. IO-threads are polling jobs from the scheduler work queue and > executing them. The connection job works in the loop. The loop is interrupted > when there is not any data to process or all IO-threads are occupied. > If the loop is interrupted then the IO-thread pushes the connection job back > into the queue and picks the next job from the queue. It should ensure that > any job is not abandoned and it is not waiting in the queue for ever. > But when the number of jobs equals to the thread pool size then IO-thread > stops processing the connection job, pushes it back into the queue and polls > the same job again, again and again. > The connection job could check whether is there any job in the scheduler > queue. If the queue was not empty then the connection working loop would be > interrupted and the thread could pick up another job. It would simplified the > broker code significantly. -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (QPID-8534) [Broker-J] Improper interruption of connection processing loop
[ https://issues.apache.org/jira/browse/QPID-8534?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17403094#comment-17403094 ] ASF GitHub Bot commented on QPID-8534: -- mklaca edited a comment on pull request #97: URL: https://github.com/apache/qpid-broker-j/pull/97#issuecomment-875630250 Hi @alex-rufous, your statement is not correct: > The #doWork() can only be invoked multiple times if there are free IO threads to process other connections Let assume situation when there is 2 threads in the thread pool, a single selector and a single connection. The selector task occupies one thread A and the second thread B is kept busy by the connection. Based on your statement the connection working loop is interrupted because there is not any free thread. The connection is re-scheduled and picked up by the same thread B for processing again. The thread B is in loop: 1. The thread picks up the connection from the working queue. 2. The thread invokes #doWork(). 3. The thread interrupts the loop of #doWork() because there is not any free thread. 4. The connection is returned to the queue. The thread B drops and picks up the same single connection and this useless iteration repeats again and again. The correct condition should be: > The #doWork() can only be invoked multiple times if there is not any another connections/job to process The suggested change keeps the "fairness functionality": If the working queue is empty then all connections are being processed by some thread and there is not any abandoned connection. Hence, no need to interrupt #doWork() loop because every connection/message has a thread. The number of active connection is smaller than the thread pool size. If there is any waiting connection then this connection has to be queued and the working queue is not empty. The #doWork() loop is interrupted after the first call of the #doWork() method and the thread picks up the waiting connection from the queue. If there is any waiting message then it's processing will not be blocked. -- 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: dev-unsubscr...@qpid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [Broker-J] Improper interruption of connection processing loop > -- > > Key: QPID-8534 > URL: https://issues.apache.org/jira/browse/QPID-8534 > Project: Qpid > Issue Type: Improvement > Components: Broker-J >Reporter: Marek Laca >Priority: Minor > Labels: Broker, Connection, Java > > The network connection scheduler (NetworkConnectionScheduler class) contains > a thread pool. IO-threads are polling jobs from the scheduler work queue and > executing them. The connection job works in the loop. The loop is interrupted > when there is not any data to process or all IO-threads are occupied. > If the loop is interrupted then the IO-thread pushes the connection job back > into the queue and picks the next job from the queue. It should ensure that > any job is not abandoned and it is not waiting in the queue for ever. > But when the number of jobs equals to the thread pool size then IO-thread > stops processing the connection job, pushes it back into the queue and polls > the same job again, again and again. > The connection job could check whether is there any job in the scheduler > queue. If the queue was not empty then the connection working loop would be > interrupted and the thread could pick up another job. It would simplified the > broker code significantly. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (QPID-8534) [Broker-J] Improper interruption of connection processing loop
[ https://issues.apache.org/jira/browse/QPID-8534?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17384855#comment-17384855 ] ASF GitHub Bot commented on QPID-8534: -- mklaca edited a comment on pull request #97: URL: https://github.com/apache/qpid-broker-j/pull/97#issuecomment-875630250 Hi @alex-rufous, your statement is not correct: > The #doWork() can only be invoked multiple times if there are free IO threads to process other connections Let assume situation when there is 2 threads in the thread pool, a single selector and a single connection. The selector task occupies one thread A and the second thread B is kept busy by the connection. Based on your statement the connection working loop is interrupted because there is not any free thread. The connection is re-scheduled and picked up by the same thread B for processing again. The thread B is in loop: 1. The thread picks up the connection from the working queue. 2. The thread invokes #doWork(). 3. The thread interrupts the loop of #doWork() because there is not any free thread. 4. The connection is returned to the queue. The thread B drops and picks up the same single connection and this useless iteration repeats again and again. The correct condition should be: > The #doWork() can only be invoked multiple times if there is not any another connections/job to process The suggested change keeps the "fairness functionality": If the working queue is empty then all connections are being processed by some thread and there is not any abandoned connection. Hence, no need to interrupt #doWork() loop because every connection/message has a thread. If the working queue is not empty then there is a waiting connection. The #doWork() loop is interrupted after the first call of the #doWork() method and the thread picks up the waiting connection from the queue. If there is any waiting message then it's processing will not be blocked. -- 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: dev-unsubscr...@qpid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [Broker-J] Improper interruption of connection processing loop > -- > > Key: QPID-8534 > URL: https://issues.apache.org/jira/browse/QPID-8534 > Project: Qpid > Issue Type: Improvement > Components: Broker-J >Reporter: Marek Laca >Priority: Minor > Labels: Broker, Connection, Java > > The network connection scheduler (NetworkConnectionScheduler class) contains > a thread pool. IO-threads are polling jobs from the scheduler work queue and > executing them. The connection job works in the loop. The loop is interrupted > when there is not any data to process or all IO-threads are occupied. > If the loop is interrupted then the IO-thread pushes the connection job back > into the queue and picks the next job from the queue. It should ensure that > any job is not abandoned and it is not waiting in the queue for ever. > But when the number of jobs equals to the thread pool size then IO-thread > stops processing the connection job, pushes it back into the queue and polls > the same job again, again and again. > The connection job could check whether is there any job in the scheduler > queue. If the queue was not empty then the connection working loop would be > interrupted and the thread could pick up another job. It would simplified the > broker code significantly. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (QPID-8534) [Broker-J] Improper interruption of connection processing loop
[ https://issues.apache.org/jira/browse/QPID-8534?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17376590#comment-17376590 ] ASF GitHub Bot commented on QPID-8534: -- mklaca commented on pull request #97: URL: https://github.com/apache/qpid-broker-j/pull/97#issuecomment-875630250 Hi @alex-rufous, your statement is not correct: > The #doWork() can only be invoked multiple times if there are free IO threads to process other connections Let assume situation when there is 2 threads in the thread pool, a single selector and a single connection. The selector task occupies one thread A and the second thread B is kept busy by the connection. Based on your statement the connection working loop is interrupted because there is not any free thread. The connection is re-scheduled and picked up by the same thread B for processing again. The thread B is in loop: 1. The thread picks up the connection from the working queue. 2. The thread invokes #doWork(). 3. The thread interrupts the loop of #doWork() because there is not any free thread. 4. The connection is returned to the queue. The thread B drops and picks up the single connection and this useless iteration repeats again and again. The correct condition should be: > The #doWork() can only be invoked multiple times if there is not any another connections/job to process The suggested change keeps the "fairness functionality": If the working queue is empty then all connections are being processed by some thread and there is not any abandoned connection. Hence, no need to interrupt #doWork() loop. If the working queue is not empty then there is a waiting connection. The #doWork() loop is interrupted after the first call of the #doWork() method and the thread picks up the waiting connection from the queue. If there is any waiting message then it's processing will not be blocked. -- 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: dev-unsubscr...@qpid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [Broker-J] Improper interruption of connection processing loop > -- > > Key: QPID-8534 > URL: https://issues.apache.org/jira/browse/QPID-8534 > Project: Qpid > Issue Type: Improvement > Components: Broker-J >Reporter: Marek Laca >Priority: Minor > Labels: Broker, Connection, Java > > The network connection scheduler (NetworkConnectionScheduler class) contains > a thread pool. IO-threads are polling jobs from the scheduler work queue and > executing them. The connection job works in the loop. The loop is interrupted > when there is not any data to process or all IO-threads are occupied. > If the loop is interrupted then the IO-thread pushes the connection job back > into the queue and picks the next job from the queue. It should ensure that > any job is not abandoned and it is not waiting in the queue for ever. > But when the number of jobs equals to the thread pool size then IO-thread > stops processing the connection job, pushes it back into the queue and polls > the same job again, again and again. > The connection job could check whether is there any job in the scheduler > queue. If the queue was not empty then the connection working loop would be > interrupted and the thread could pick up another job. It would simplified the > broker code significantly. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (QPID-8534) [Broker-J] Improper interruption of connection processing loop
[ https://issues.apache.org/jira/browse/QPID-8534?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17374329#comment-17374329 ] Alex Rudyy commented on QPID-8534: -- I do not think that this JIRA is correct. The reason why connections `#doWork` is mainly invoked once is to keep "fairness", especially when all IO threads are used. Though, that might result in more frequent connection re-scheduling even for situations when there are data to read or write, the current implementation guarantees some level of fairness - each connection with "work" would have its chance to get invoked. Otherwise, the connections with more work (big messages or more frequent IO operations) would be dominating IO threads and blocking other connections with smaller messages or less frequent IO operations. Thus, the current implementation is by design. I agree that implemented code is complex. I do not mind re-factoring to make it simpler. However, it needs to be done with keeping existing behaviour including fairness... > [Broker-J] Improper interruption of connection processing loop > -- > > Key: QPID-8534 > URL: https://issues.apache.org/jira/browse/QPID-8534 > Project: Qpid > Issue Type: Improvement > Components: Broker-J >Reporter: Marek Laca >Priority: Minor > Labels: Broker, Connection, Java > > The network connection scheduler (NetworkConnectionScheduler class) contains > a thread pool. IO-threads are polling jobs from the scheduler work queue and > executing them. The connection job works in the loop. The loop is interrupted > when there is not any data to process or all IO-threads are occupied. > If the loop is interrupted then the IO-thread pushes the connection job back > into the queue and picks the next job from the queue. It should ensure that > any job is not abandoned and it is not waiting in the queue for ever. > But when the number of jobs equals to the thread pool size then IO-thread > stops processing the connection job, pushes it back into the queue and polls > the same job again, again and again. > The connection job could check whether is there any job in the scheduler > queue. If the queue was not empty then the connection working loop would be > interrupted and the thread could pick up another job. It would simplified the > broker code significantly. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (QPID-8534) [Broker-J] Improper interruption of connection processing loop
[ https://issues.apache.org/jira/browse/QPID-8534?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17367505#comment-17367505 ] ASF GitHub Bot commented on QPID-8534: -- mklaca opened a new pull request #97: URL: https://github.com/apache/qpid-broker-j/pull/97 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [Broker-J] Improper interruption of connection processing loop > -- > > Key: QPID-8534 > URL: https://issues.apache.org/jira/browse/QPID-8534 > Project: Qpid > Issue Type: Improvement > Components: Broker-J >Reporter: Marek Laca >Priority: Minor > Labels: Broker, Connection, Java > > The network connection scheduler (NetworkConnectionScheduler class) contains > a thread pool. IO-threads are polling jobs from the scheduler work queue and > executing them. The connection job works in the loop. The loop is interrupted > when there is not any data to process or all IO-threads are occupied. > If the loop is interrupted then the IO-thread pushes the connection job back > into the queue and picks the next job from the queue. It should ensure that > any job is not abandoned and it is not waiting in the queue for ever. > But when the number of jobs equals to the thread pool size then IO-thread > stops processing the connection job, pushes it back into the queue and polls > the same job again, again and again. > The connection job could check whether is there any job in the scheduler > queue. If the queue was not empty then the connection working loop would be > interrupted and the thread could pick up another job. It would simplified the > broker code significantly. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org