Exec connection #852
Exec connection #852
Conversation
| ApplicationMessage message = (ApplicationMessage) record.getFragment(); | ||
| session.markRecordAsRead(record.getEpoch(), record.getSequenceNumber()); | ||
| // create application message. | ||
| RawData receivedApplicationMessage = RawData.inbound(message.getData(), session.getConnectionReadContext(), false); |
There was a problem hiding this comment.
receivedApplicationMessage can not be null ?
There was a problem hiding this comment.
I see only possible Exceptions, but no null.
| executor.shutdownNow(pending); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Once a ConnectionStore is stopped, it can never be restarted, right ?
There was a problem hiding this comment.
At least for unitests the "restart" is used. Therefore the connections kept in the store. See markAllAsResumptionRequired call in start(InetSocketAddress).
| if (serialExecutor != null) { | ||
| serialExecutor.execute(new Runnable() { | ||
| SerialExecutor serialExecutor = connection.getExecutor(); | ||
| serialExecutor.execute(new Runnable() { |
There was a problem hiding this comment.
serialExecutor is never null ? because a connection still have an alive executor ?
There was a problem hiding this comment.
Only "ticket" connections use a null SerialExecutor, but they re only returned in find(SessionId).
| } | ||
| } else if (!connection.isExecuting() && running.get()) { | ||
| LOGGER.debug("Recreate new connection for {}", connection); | ||
| newConnection = new Connection(connection, new SerialExecutor(executor)); |
There was a problem hiding this comment.
should we check create parameter for recreate ?
There was a problem hiding this comment.
Here we have a connection (may be after restart), but we need one with a active executor. "create" means a really new connection not only a revived one.
| * If the provided connection has either a handshaker or a established | ||
| * session, create a new connection. | ||
| * | ||
| * @param connection connection |
There was a problem hiding this comment.
The behavior of this method is not so clear to me, we give a connection and we get a new one. Maybe more javadoc could help.
missing removeFromSessionCache parameter which has not an obvious utility in this case.
There was a problem hiding this comment.
The method returns a "fresh active connection" without handshaker nor session and an active executor. The missing javadoc will be added.
| @@ -1356,11 +1451,13 @@ private void startNewHandshake(final ClientHello clientHello, final Record recor | |||
| initializeHandshaker(handshaker); | |||
| handshaker.processMessage(record); | |||
| } | |||
| } | |||
| error = new IllegalStateException("connector's serial-executors-cache is exhausted!"); | ||
| connection = getAliveConnection(msg.getInetSocketAddress(), !serverOnly); | ||
| if (connection == null) { | ||
| if (connectionStore.remainingCapacity() > 0 && serverOnly) { |
There was a problem hiding this comment.
if connectionStore.remainingCapacity() > 0 and we are not in serverOnly mode we raise new IllegalStateException("connection store is exhausted!"), is it normal ?
There was a problem hiding this comment.
If serverOnly is false, new connections are created, and so null is returned, if the the store is exhausted and the put failed.
If serverOnly is true, the remaining call is not required. I will remove it.
| connection.cancelPendingFlight(); | ||
| synchronized (connectionStore) { |
There was a problem hiding this comment.
should we add a new atomic function in connectionStore ? (e.g. replace ?)
There was a problem hiding this comment.
May be at the end of the "connection id wip".
| @@ -1839,15 +1937,13 @@ protected void sendNextDatagramOverNetwork(final DatagramPacket datagramPacket) | |||
| } | |||
| } | |||
|
|
|||
| private void handleTimeout(DTLSFlight flight) { | |||
| private void handleTimeout(DTLSFlight flight, Connection connection) { | |||
There was a problem hiding this comment.
some formatting issue in this method
boaks
force-pushed
the
exec_connection
branch
4 times, most recently
from
7253098
to
c542d75
Compare
|
I cleaned up the formatting of the changes. Though some of the stuff is changed again with the other parts of the related wip, I would prefer to have that part merged. |
|
👍 |
Decouple serial execution from InetSocketAddress to prepare for connection id support. Remove synchronization on dtls session. With the introduction of serialized execution based on the connection, synchronization based on the dtls session is obsolete. Signed-off-by: Achim Kraus <achim.kraus@bosch-si.com>
boaks
force-pushed
the
exec_connection
branch
from
c542d75
to
52b5eee
Compare
Remove synchronization on dtls session and move serial executor fro inet socket address into connection. Preparation for connection id support.
Signed-off-by: Achim Kraus achim.kraus@bosch-si.com