Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal for Early Connector Start #10664

Closed
wants to merge 2 commits into from
Closed

Proposal for Early Connector Start #10664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
021040f
Proposal for Early Connector Start
joakime Oct 4, 2023
941b39e
Set startConnectorsEarly to default of false
joakime Oct 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 41 additions & 13 deletions jetty-server/src/main/java/org/eclipse/jetty/server/Server.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ public class Server extends HandlerWrapper implements Attributes
private final AutoLock _dateLock = new AutoLock();
private volatile DateField _dateField;
private long _stopTimeout;
private boolean _startConnectorsEarly = false;

public Server()
{
Expand Down Expand Up @@ -133,6 +134,16 @@ public Server(@Name("threadpool") ThreadPool pool)
setServer(this);
}

public void setStartConnectorsEarly(boolean earlyConnectors)
{
_startConnectorsEarly = earlyConnectors;
}

public boolean isStartConnectorsEarly()
{
return _startConnectorsEarly;
}

public boolean isDryRun()
{
return _dryRun;
Expand Down Expand Up @@ -410,6 +421,13 @@ protected void doStart() throws Exception
mex.ifExceptionThrow();
}

if (_startConnectorsEarly)
{
LOG.info("Starting Connectors Early");
// start connectors
startConnectors(mex);
}

// Start the server and components, but not connectors!
// #start(LifeCycle) is overridden so that connectors are not started
super.doStart();
Expand All @@ -420,22 +438,13 @@ protected void doStart() throws Exception
throw new StopException();
}

// start connectors
for (Connector connector : _connectors)
if (!_startConnectorsEarly)
{
try
{
connector.start();
}
catch (Throwable e)
{
mex.add(e);
// stop any started connectors
_connectors.stream().filter(LifeCycle::isRunning).map(Object.class::cast).forEach(LifeCycle::stop);
}
LOG.info("Starting Connectors Late");
// start connectors
startConnectors(mex);
}

mex.ifExceptionThrow();
LOG.info(String.format("Started %s @%dms", this, Uptime.getUptime()));
}
catch (Throwable th)
Expand All @@ -462,6 +471,25 @@ protected void doStart() throws Exception
}
}

private void startConnectors(MultiException mex) throws Exception
{
for (Connector connector : _connectors)
{
try
{
connector.start();
}
catch (Throwable e)
{
mex.add(e);
// stop any started connectors
_connectors.stream().filter(LifeCycle::isRunning).map(Object.class::cast).forEach(LifeCycle::stop);
}
}

mex.ifExceptionThrow();
}

@Override
protected void start(LifeCycle l) throws Exception
{
Expand Down
2 changes: 1 addition & 1 deletion jetty-server/src/main/java/org/eclipse/jetty/server/handler/HandlerList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
{
Handler[] handlers = getHandlers();

if (handlers != null && isStarted())
Copy link
Contributor Author

@joakime joakime Oct 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is potentially controversial, as in order for the example in the PR to work, this isStarted() check cannot be here.

This seems arbitrary and not all Handler do this check.
Eg: ContextHandlerCollection and GzipHandler do not check isStarted(), BUT HandlerList, HandlerCollection, and RewriteHandler do.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that checking is a bit arbitrary, but the check should be valid non-the-less

if (handlers != null)
{
for (int i = 0; i < handlers.length; i++)
{
Expand Down