airavata-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Saminda Wijeratne <samin...@gmail.com>
Subject Re: Orchestration Component implementation review
Date Mon, 20 Jan 2014 05:38:50 GMT
On Sun, Jan 19, 2014 at 2:46 PM, Lahiru Gunathilake <glahiru@gmail.com>wrote:

>
>
>
> On Fri, Jan 17, 2014 at 1:29 PM, Amila Jayasekara <thejaka.amila@gmail.com
> > wrote:
>
>>
>>
>>
>> On Fri, Jan 17, 2014 at 10:32 AM, Saminda Wijeratne <samindaw@gmail.com>wrote:
>>
>>> Following are few thoughts I had during my review of the component,
>>>
>>> *Multi-threaded vs single threaded*
>>> If we are going to have multi-threaded job submission the implementation
>>> should work on handling race conditions. Essentially JobSubmitter should be
>>> able to "lock" an experiment request before continuing processing that
>>> request so that other JobSubmitters accessing the experiment requests a the
>>> same time would skip it.
>>>
>>
>> +1. These are implementation details.
>>
>>
>>>
>>> *Orchestrator service*
>>> We might want to think of the possibility in future where we will be
>>> having multiple deployments of an Airavata service. This could particularly
>>> be true for SciGaP. We may have to think how some of the internal data
>>> structures/SPIs should be updated to accomodate such requirements in future.
>>>
>>
>> +1.
>>
>>
>>>
>>> *Orchestrator Component configurations*
>>> I see alot of places where the orchestrator can have configurations. I
>>> think its too early finalize them, but I think we can start refactoring
>>> them out perhaps to the airavata-server.properties. I'm also seeing the
>>> orchestrator is now hardcoded to use default/admin gateway and username. I
>>> think it should come from the request itself.
>>>
>>
>> +1. But in overall we may need to change the way we handle configurations
>> within Airavata. Currently we have multiple configuration files and
>> multiple places where we read configurations. IMO we should have a separate
>> module to handle configurations. Only this module should be aware how to
>> intepret configurations in the file and provide a component interface to
>> access those configuration values.
>>
> I like this approach, this will work with deploying components separate
> (if we are planning to do that). We can come to an approach like we have in
> Airavata API (Managers), like we have ServerSetting might composed of
> multiple settings (GFACsettings, OrchestratorSettings). So during isolated
> deployments some Settings objects could be null.
>
We can use the approach we use for RegistrySettings. When a
"registry.properties" exists it will pick up the registry configuration
properties from that file or else It will pick up from
airavata-server.properties/airavata-client.properties.

>
>>
>>>
>>> *Visibility of API functions*
>>> I think initialize(), shutdown() and startJobSubmitter() functions
>>> should not be part of the API because I don't see a scenario where the
>>> gateway developer would be responsible for using them. They serve a more
>>> internal purpose of managing the orchestrator component IMO. As Amila
>>> pointed out so long ago (wink) functions that do not concern outside
>>> parties should not be used as part of the API.
>>>
>>
>> +1
>>
>>
>>>
>>> *Return values of Orchestrator API*
>>> IMO unless it is specifically required to do so I think the functions
>>> does not necessarily need to return anything other than throw exceptions
>>> when needed. For example the launchExperiment can simply return void if all
>>> is succesful and return an exception if something fails. Handling issues
>>> with a try catch is not only simpler but also the explanations are readily
>>> available for the user.
>>>
>>
>> +1. Also try to have different exception for different scenarios. For
>> example if persistence (hypothetical) fails, DatabasePersistenceException,
>> if validation fails, ValidationFailedException etc ... Then the developer
>> who uses the API can catch these different exceptions and act on them
>> appropriately.
>>
>>
>>>
>>> *Data persisted in registry*
>>> ExperimentRequest.getUsername() : I think we should clarify what this
>>> username denotes. In current API, in experiment submission we consider two
>>> types of users. Submission user (the user who submits the experiment to the
>>> Airavata Server - this is inferred by the request itself) and the execution
>>> user (the user who corelates to the application executions of the gateway -
>>> thus this user can be a different user for different gateway, eg: community
>>> user, gateway user).
>>> I think we should persist the date/time of the experiment request as
>>> well.
>>>
>> +1
>>
>>>  Also when retrying of API functions in the case of a failure in an
>>> previous attempt there should be a way to not to repeat already performed
>>> steps or gracefully roleback and redo those required steps as necessary.
>>> While such actions could be transparent to the user sometimes it might make
>>> sense to allow user to be notified of success/failure of a retry. However
>>> this might mean keeping additional records at the registry level.
>>>
>>
>> In addition we should also have a way of cleaning up unsubmitted
>> experiment ids. (But not sure whether you want to address this right now).
>> The way I see this is to have a periodic thread which goes through the
>> table and clear up experiments which are not submitted for a defined time.
>>
> +1
>
> Lahiru
>
>>
>> BTW, nice review notes, Saminda.
>>
>> Thanks
>> Amila
>>
>>
>>
>
>
> --
> System Analyst Programmer
> PTI Lab
> Indiana University
>

Mime
View raw message