Bug 362394 - Extensible Server Start Jobs
Summary: Extensible Server Start Jobs
Status: CLOSED FIXED
Alias: None
Product: WTP ServerTools
Classification: WebTools
Component: wst.server (show other bugs)
Version: 3.3   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 3.4   Edit
Assignee: Naci Dai CLA
QA Contact: Elson Yuen CLA
URL:
Whiteboard: PMC_approved
Keywords: api
Depends on:
Blocks: 370992 362393 367787
  Show dependency tree
 
Reported: 2011-10-29 07:20 EDT by Naci Dai CLA
Modified: 2017-10-11 16:37 EDT (History)
4 users (show)

See Also:


Attachments
Patch to add sychronous start capability to servers (4.76 KB, patch)
2011-10-30 06:55 EDT, Naci Dai CLA
naci.dai: review?
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Naci Dai CLA 2011-10-29 07:20:51 EDT
Existing WTP Server Start/Debug in the Server class creates a StartJob and a
PublishJob and runs them async.  

OSGi launchers typically create a framework configuration as a part of the
publish process.  So it is desirable to first publish then start (i.e. wait
until the publish is completed).  JavaEE servers prefer that the server is
started before a publish process.  Currently this is hardwired, there is no
extension mechanism to change it for a server adapter.  OSGİ Run/Debug does it
in the reverse order.
Comment 1 Naci Dai CLA 2011-10-29 07:22:16 EDT
This bug  is related to implementing the Run OSGi feature in Libra.

Related Bug 362393
Comment 2 Naci Dai CLA 2011-10-30 06:55:04 EDT
Created attachment 206178 [details]
Patch to add sychronous start capability to servers

This patch adds a new optional boolean attribute  to ServerType  called synchronousStart with default value of false to preserve the current behavior for Servers
Comment 3 Naci Dai CLA 2011-10-30 06:59:53 EDT
The proposed non-breaking change to preserve the behavior for existing server adapters  is as follows:

1) serverType.exsd extension Point: Add an optional NEW boolean attribute to server types "synchronousStart". Default value is false (current behavior)

2) Add an accessor to ServerType.java to return the value of the element

3) Modify Server.java "start" methods to use the value of synchronousStart attribute.  If the attribute is false (default) the behavior is unchanged, if it is true, the start methods acts the same as the synchrnousStart method.
Comment 4 Naci Dai CLA 2011-11-14 08:51:26 EST
seeking PMC Approval to commit this change to 3.4 stream
Comment 5 Naci Dai CLA 2011-11-15 08:36:07 EST
I am filling a questionnaire incase one is needed to approve this enhancement.

*Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug"
(requested by an adopter) please document it as such.

It is not a hotbug. I am a committer for server tools. This is an enhancement.

*Is there a work-around? If so, why do you believe the work-around is
insufficient?

No workaround.

*How has the fix been tested? Is there a test case attached to the bugzilla
record? 

Ad-hoc testing has been done.

*Has a JUnit Test been added?

A new test has not been added. Existing server tests pass. 

*Give a brief technical overview. Who has reviewed this fix?

Existing WTP Server Start/Debug in the Server class creates a StartJob and a
PublishJob and runs them async.  This patch adds a new optional boolean attribute  to ServerType  called synchronousStart with default value of false to preserve the current behavior
for Servers.  A review was requested at wtp-dev, but none took place so far. 

*What is the risk associated with this fix?

Low.  Existing behavior is preserved.
Comment 6 Chuck Bridgham CLA 2011-11-15 09:44:05 EST
This change looks good, and is safe.  I approve
Comment 7 Elson Yuen CLA 2011-11-15 11:34:27 EST
Naci, can you clarify the scenario that you are trying to address?  Based on what you have described, you are trying to introduce a way to publish the server first and then do a server start instead of starting a server before a publish occurs.

The serverType extension point today has the startBeforePublish flag that allows the adopter to specify that behaviour already.  If that flag is set to false, then the framework will publish the server before starting the server.  How is that different from the proposed new synchronousStart flag?
Comment 8 Naci Dai CLA 2011-12-09 02:46:15 EST
Our server adapters for OSGi launcher already specify  startBeforePublish="false".  Although this starts the PublishJob before the StartJob they are still async and does not achieve the same behavior.  Start Job does not wait for the publish to complete before it is complete.  This behavior is achieved with the syncStart patch.
Comment 9 Naci Dai CLA 2011-12-10 04:19:25 EST
startBeforePublish attribute forces a server to start (if it is not already running).  This  behavior is needed when a server must be running so that server adapter can make some api calls to it for the publish job.   setting it to false does not achieve "publishBeforeStart" behavior.

 Also, this attribute does not regulate how start/publish jobs are managed.  Therefore it is orthogonal to the new synchronousStart attribute.
Comment 10 David Williams CLA 2011-12-12 16:26:12 EST
(In reply to comment #8)
> Our server adapters for OSGi launcher already specify 
> startBeforePublish="false".  Although this starts the PublishJob before the
> StartJob they are still async and does not achieve the same behavior.  Start
> Job does not wait for the publish to complete before it is complete.  This
> behavior is achieved with the syncStart patch.

Is this the intended behavior? Seems counterintuitive to me. But ... perhaps there is a reason or use-case for that behavior? Performance (or, perception of performance)?  

Or, does it just happens to be that way, since forever; if so I'd assume it would be unwise to change behavior after all these years ... just in case has unintended consequences?

So, in either case, adding an attribute to further define behavior would seem to make sense.
Comment 11 Elson Yuen CLA 2011-12-23 14:03:34 EST
Naci, thanks for the explanation.  The new changes makes sense to me now.  The changes looks good to me.
Comment 12 Elson Yuen CLA 2012-03-08 14:12:15 EST
Naci, I think the changes has already went in.  Can you mark this defect as resolved in the changes are already in? Thanks.
Comment 13 Naci Dai CLA 2012-03-15 06:16:37 EDT
Changes are commited in Juno builds
Comment 14 Naci Dai CLA 2012-03-15 06:19:26 EDT
closing
Comment 15 Eclipse Genie CLA 2017-10-11 16:37:29 EDT
New Gerrit change created: https://git.eclipse.org/r/109082
Comment 16 Eclipse Genie CLA 2017-10-11 16:37:31 EDT
New Gerrit change created: https://git.eclipse.org/r/109081
Comment 17 Eclipse Genie CLA 2017-10-11 16:37:48 EDT
New Gerrit change created: https://git.eclipse.org/r/109088