Jms connection status #1252

andreas-schroeder · 2018-09-30T09:05:26Z

This is my proposal for JMS connection status.

andreas-schroeder · 2018-09-30T09:07:01Z

This PR extends #1227, don't merge this before the former is merged/approved.

andreas-schroeder · 2018-09-30T15:26:28Z

Finally, travis failed with the test I expeced to fail (due to re-connection failing): https://travis-ci.org/akka/alpakka/jobs/435270299#L690

andreas-schroeder · 2018-10-01T19:15:22Z

Commit 745a6ef fixes an issue in the test mentioned here: #1231 - However, a different error message.

andreas-schroeder · 2018-10-05T04:35:01Z

rebased on top of master

2m · 2018-10-06T14:44:32Z

I am wondering if exposing a boolean as a connectivity status is useful enough.

An alternative would be to expose a Source[JmsConnectorState, ...] instead. From that it would be possible for the user to create a stream that consumers state changes and updates an atomic reference if that is what is needed.

WDYT?

andreas-schroeder · 2018-10-07T11:32:53Z

The boolean connectivity status would be enough for my use case, as I don't need immediate connectivity updates - having a delay of 1 second would be fine for me.

That said, I see having a stream of connectivity updates could also be beneficial if a fast response to connectivity changes is needed, so let's explore how this could look like.

How would you suggest expose the Source? As part of the materialized value, or by using a fan-out shape in the stage?

2m · 2018-10-07T19:25:35Z

I think it should be materialized value. Together with a KillSwitch, like you have connectivity status currently.

andreas-schroeder · 2018-10-07T20:02:37Z

Hi @2m, I've spiked the change and one challenge is that the source needs to be pre-materialized, and that can only happen during preStart. Consequently, the source might not be there at the time the materialized value's connected method is called, and I either need to busy wait for the preStart to complete or return a Future[Source[...]]. I like to keep the API simple, but I guess that to stay in line with the non-blocking mantra of Akka, we need to go for the Future - what do you think?

2m · 2018-10-07T20:46:17Z

How are you building the status source?

If it is, for example, a Source.queue then you can prematerialize the source to get out the materialized value, which now can be passed into the graph stage.

Alternatively, to convert from Future[Source[T, ...]] to Source[T, ...] you can do:

val fs = Future[Source[T, ...]]
val s: Source[T, ...] = Source.fromFuture(fs).flatMapConcat(identity)

andreas-schroeder · 2018-10-09T17:24:19Z

@2m the Future conversion is elegant, will do! 👍It will take me a couple to assemble something to show; the Source and the JmsConnectorState types needs to be Java-ified for the Java-DSL, and the exposed state shouldn't have the JmsConnection exposed either. So there's a couple lines of code to write there.

andreas-schroeder · 2018-10-17T18:20:03Z

@2m here we go - I rebased on master and squashed everything into one commit. Have a look and tell me what you think.

I will add a couple more tests for connection status, and for correct handling of reconnection failures on each stage (creating connection, session, consumer/producer, and destination).

andreas-schroeder · 2018-10-18T15:10:15Z

Here are some more tests. I was thinking to replace the status update Stopping with Failed and Completed respectively, but that is a bit more effort than just having Stopping. What do you think, @2m ?

andreas-schroeder · 2018-10-21T10:12:38Z

So I went ahead and did the split of "stopping" into "failing" and "completing". I've also updated the docs and updated all factory methods in JmsProducer and JmsConsumer.

For the Java-API, this is a breaking change.
For the Scala-API, this is a non-breaking change.

This PR is complete from my side, please have a look.

Stream-based connection status Connection status is now exposed as stream of updates. For the Java interface, a Java enum is exposed, for the Scala interface, a sealed trait with case objects and a case class is used. Fix reconnect attempts Now also retry to connect (for a limited amount of times and after delay) if creating sessions, consumers, producers or destinations fail Fix flaky 'disconnect exceptional completion' test Fixes akka#1231 Fixes akka#1031

- Add tests for connection status and reconnects on session creation failures - Connecting status attempt number was off-by-one for JmsConsumer - Session creation failure was not properly handled

- Also made sure that published states are natural: no intermediate disconnected state is published on failure - Also improved logic handling consumer downstream completion - Also ensured that access to state and state queue is safe - Added more tests for connection status

- Breaking change: using JmsConsumerControl and JmsProducerStatus in all JmsProducer and JmsConsumer factory methods - Adjusted and extended docs - Also renamed stage state source to connectorState - Also fixed flaky test "abort connection on security exception" - Also simplified openConnection method

- Reverted openConnection simplification, as it did not handle exceptions on connection.setExceptionListener - Added test that demonstrates the simplification was invalid - Removed unnecessary broker forced restart in Spec

andreas-schroeder · 2018-10-27T17:14:51Z

rebased once more on top of master.

2m

LGTM! Great work @andreas-schroeder!

andreas-schroeder · 2018-10-29T18:28:31Z

Thanks @2m - also for your input, I think this PR wouldn't be in that shape without your ideas.

ennru

LGTM.

ennru · 2018-10-30T08:14:20Z

jms/src/main/scala/akka/stream/alpakka/jms/javadsl/package.scala

+      source: stream.scaladsl.Source[jms.scaladsl.JmsConnectorState, NotUsed]
+  ): Source[JmsConnectorState, NotUsed] =
+    source.map {
+      case Disconnected => JmsConnectorState.Disconnected


I guess the Scala states could contain the Java state and offer asJava.

Ok, that introduce a dependency from the scaladsl to the javadsl, would that be fine for you?

Yes, even if the dependency goes the other direction for most cases.

Implemented with 6a406b7

ennru · 2018-10-30T18:08:58Z

Thank you for your continued efforts with the JMS connector.

andreas-schroeder · 2018-10-30T18:40:11Z

Thank you for keeping everything nicely together! Really appreciate the efforts you and your team put into Alpakka.

Btw, this PR also fixed the test failures mentioned in issues #1031 and #1231.

sullis · 2018-10-30T18:50:07Z

when is the next Milestone release?

andreas-schroeder mentioned this pull request Oct 2, 2018

Jms producer retries #1227

Merged

ennru added the p:jms label Oct 4, 2018

andreas-schroeder force-pushed the jms-connection-status branch from 745a6ef to 92b1192 Compare October 5, 2018 04:34

andreas-schroeder force-pushed the jms-connection-status branch from 92b1192 to 13b0456 Compare October 7, 2018 12:42

andreas-schroeder force-pushed the jms-connection-status branch 2 times, most recently from f59e84f to 6896007 Compare October 17, 2018 16:54

andreas-sa-schroeder added 8 commits October 27, 2018 19:13

Add more connection status tests and minor fixes

8457a7b

- Add tests for connection status and reconnects on session creation failures - Connecting status attempt number was off-by-one for JmsConsumer - Session creation failure was not properly handled

Delete zombie JmsSource.scala

6082ace

Complete State source when stage stops, consolidate method names

8590a82

Fix jms doc-examples compilation

de2d2f7

Fix openConnection behavior under failure

82b235a

- Reverted openConnection simplification, as it did not handle exceptions on connection.setExceptionListener - Added test that demonstrates the simplification was invalid - Removed unnecessary broker forced restart in Spec

andreas-schroeder force-pushed the jms-connection-status branch from e9d2889 to 82b235a Compare October 27, 2018 17:13

2m added the api-change label Oct 28, 2018

2m approved these changes Oct 28, 2018

View reviewed changes

ennru approved these changes Oct 30, 2018

View reviewed changes

Convert from scaladsl to javadsl via .asJava

6a406b7

ennru merged commit ba8bee3 into akka:master Oct 30, 2018

ennru added this to the 1.0-M2 milestone Oct 30, 2018

This was referenced Oct 31, 2018

Failed: JmsConnectorsSpec sink disconnect exceptional completion #1031

Closed

Failed: JMS Connectors should sink disconnect exceptional completion #1231

Closed

ennru mentioned this pull request Nov 14, 2018

Jms does not handle failures #201

Closed

2m modified the milestones: 1.0-M2, 1.0-M1 Jan 17, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jms connection status #1252

Jms connection status #1252

andreas-schroeder commented Sep 30, 2018

andreas-schroeder commented Sep 30, 2018

andreas-schroeder commented Sep 30, 2018

andreas-schroeder commented Oct 1, 2018

andreas-schroeder commented Oct 5, 2018

2m commented Oct 6, 2018

andreas-schroeder commented Oct 7, 2018

2m commented Oct 7, 2018

andreas-schroeder commented Oct 7, 2018

2m commented Oct 7, 2018

andreas-schroeder commented Oct 9, 2018

andreas-schroeder commented Oct 17, 2018

andreas-schroeder commented Oct 18, 2018

andreas-schroeder commented Oct 21, 2018

andreas-schroeder commented Oct 27, 2018

2m left a comment

andreas-schroeder commented Oct 29, 2018

ennru left a comment

ennru Oct 30, 2018

andreas-schroeder Oct 30, 2018

ennru Oct 30, 2018

andreas-schroeder Oct 30, 2018

ennru commented Oct 30, 2018

andreas-schroeder commented Oct 30, 2018

sullis commented Oct 30, 2018

Jms connection status #1252

Jms connection status #1252

Conversation

andreas-schroeder commented Sep 30, 2018

andreas-schroeder commented Sep 30, 2018

andreas-schroeder commented Sep 30, 2018

andreas-schroeder commented Oct 1, 2018

andreas-schroeder commented Oct 5, 2018

2m commented Oct 6, 2018

andreas-schroeder commented Oct 7, 2018

2m commented Oct 7, 2018

andreas-schroeder commented Oct 7, 2018

2m commented Oct 7, 2018

andreas-schroeder commented Oct 9, 2018

andreas-schroeder commented Oct 17, 2018

andreas-schroeder commented Oct 18, 2018

andreas-schroeder commented Oct 21, 2018

andreas-schroeder commented Oct 27, 2018

2m left a comment

Choose a reason for hiding this comment

andreas-schroeder commented Oct 29, 2018

ennru left a comment

Choose a reason for hiding this comment

ennru Oct 30, 2018

Choose a reason for hiding this comment

andreas-schroeder Oct 30, 2018

Choose a reason for hiding this comment

ennru Oct 30, 2018

Choose a reason for hiding this comment

andreas-schroeder Oct 30, 2018

Choose a reason for hiding this comment

ennru commented Oct 30, 2018

andreas-schroeder commented Oct 30, 2018

sullis commented Oct 30, 2018