BZ966777 and interruptible component invocation

Thomas Segismont tsegismo at redhat.com
Mon Jul 1 12:32:04 UTC 2013


Lukas,

Please find my answers below

Le 01/07/2013 13:08, Lukas Krejci a écrit :
> I have one question and one comment :)
>
> 1) What is the advantage of ComponentInvocationContext over
> Thread.isInterrupted() and Thread.interrupt()?

ComponentInvocationContext is an RHQ concept and has very few chances to 
get messed up.

This is not the case with Thread interruption. Many plugin component 
implementations use libraries and these libraries could clear the 
interrupted status before the component gets a chance to check it.



>
> 2) I'd personally split the ComponentInvocationContext into 2 classes - 1
> visible in the plugin API and the other only in the plugin container.
> I think ComponentInvocationContext.remove() and setCurrent() methods should
> not be used by the plugins or have I misunderstood their purpose?
>

Right it's not intended that plugins use 
ComponentInvocationContext.remove() and setCurrent().

If we want to do this we could have a new 
getComponentInvocationContext() in ResourceContext class.

Any other idea?


> On Monday, July 01, 2013 10:10:30 Thomas Segismont wrote:
>> Hi,
>>
>> I've pushed my changes to a bug branch. Can someone please have a look
>> at it before it gets merged?
>>
>> https://git.fedorahosted.org/cgit/rhq/rhq.git/commit/?h=bug/966777&id=7b7f0e
>> ff22213847156e9262090f65f845842185
>>
>> This first commit adds a new transferInterrupt parameter to
>> ResourceContainer#createResourceComponentProxy
>>
>> Here is the Javadoc:
>>
>> +     * @param transferInterrupt whether or not interruption of the
>> calling thread should be transferred to the executor
>> +     *                          thread
>>
>> https://git.fedorahosted.org/cgit/rhq/rhq.git/commit/?h=bug/966777&id=a6c62d
>> 9bd0c8d848c2770892872f4ee200e059f0
>>
>> The second commit introduces the ComponentInvocationContext class. An
>> instance of this class is created by the plugin container and bound to
>> facet-locked component invocation thread.
>>
>> This commit also fixes the actual problem of BZ966777 (EAP 6 plug-in is
>> using a hard-coded operation timeout for start and stop).
>>
>> Thanks and regards,
>> Thomas
>>
>> Le 17/06/2013 17:25, Thomas Segismont a écrit :
>>> Hi everyone,
>>>
>>> I'm working on BZ966777 [1]: "EAP 6 plug-in is using a hard-coded
>>> operation timeout for start and stop instead of using the operation
>>> timeout or agent's default operation timeout of 10 minutes"
>>>
>>> The reason why the start operation of this plugin is failing after 15
>>> seconds is that the implementation of BaseServerComponent#startServer
>>> creates a ProcessExecution and then calls
>>> "processExecution.setWaitForCompletion(15000L);"
>>>
>>> Removing this call is necessary as part of the fix but it won't be
>>> enough. After the SystemInfo instance has been asked to execute the
>>> process, it is checked if the AS7 server has started in
>>> BaseServerComponent#waitForServerToStart. This method tries up to 20
>>> times to read an attribute of the root node of the server management
>>> tree. The invocation thread sleeps one second between tries.
>>> Consequently, with slow starting servers the operation would still be
>>> marked as failed after just over 20 seconds.
>>>
>>> So I read the OperationFacet contract. Here's an extract of the
>>> OperationFacet#invokeOperation Javadoc:
>>>
>>> "Plugin developers must write implementations of this method with the
>>> awareness that operations are cancelable and should prepare for such a
>>> possibility in the code. For example, a user may request that an
>>> operation be canceled, or an operation may be taking too long and it
>>> should be canceled due to a timeout expiration. When an operation
>>> invocation is canceled, the thread that is invoking this method will be
>>> interrupted. [...] When an operation invocation is canceled, the thread
>>> that is invoking this method will be interrupted. [...] Plugin writers
>>> are encouraged to check the current thread's Thread.isInterrupted()
>>> method periodically to catch cancellation requests, as well as catching
>>> and processing InterruptedException appropriately."
>>>
>>> My understanding is that canceled and timed out operations should result
>>> in the invocation thread being interrupted. Neither is happening.
>>>
>>> OperationInvocation#markAsCanceled actually interrupts a thread from the
>>> "OperationManager.invoker" pool. But it's not this one which executes
>>> the operation, it's a thread from the
>>> "ResourceContainer.invoker.nonDaemon" pool, because the resource
>>> component is invoked through a proxy. And in
>>> ResourceComponentInvocationHandler#invokeInNewThreadWithLock, only
>>> daemon threads are interrupted when the caller thread is interrupted.
>>>
>>> So far, there's a couple of questions I wasn't able to answer:
>>>
>>> * why do we have two different pools in
>>> ResourceComponentInvocationHandler?
>>>
>>> These pools appeared in commit fb3a048 [2] and before that, every
>>> ComponentInvocationThread was a daemon thread.
>>>
>>> * why do we transfer the "interrupt" to daemon threads only?
>>>
>>> Any ideas?
>>>
>>> In addition, I think that calling interrupt to tell a component that an
>>> operation was canceled is not a strong enough design. Plugins developers
>>> use libraries to implement operations (so do we). The interrupted status
>>> of the thread could be cleared by a library. I would like to introduce a
>>> new method to the ResourceContext class:
>>>
>>> ComponentInvocationContext getComponentInvocationContext();
>>>
>>> And the ComponentInvocationContext interface:
>>>
>>> public interface ComponentInvocationContext {
>>>
>>>     boolean isCanceled();
>>>
>>> }
>>>
>>> Then plugin developers would only have to check.
>>>
>>> What's you opinion?
>>>
>>> Thanks and regards,
>>> Thomas
>>>
>>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=966777
>>> [2]
>>> https://git.fedorahosted.org/cgit/rhq/rhq.git/commit/?id=fb3a0489469062bac
>>> 9db053389481f7d80babd8b
>>>
>>> _______________________________________________
>>> rhq-devel mailing list
>>> rhq-devel at lists.fedorahosted.org
>>> https://lists.fedorahosted.org/mailman/listinfo/rhq-devel
>>
>> _______________________________________________
>> rhq-devel mailing list
>> rhq-devel at lists.fedorahosted.org
>> https://lists.fedorahosted.org/mailman/listinfo/rhq-devel
> _______________________________________________
> rhq-devel mailing list
> rhq-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/rhq-devel
>



More information about the rhq-devel mailing list