Spring boot sample #45
Conversation
| @@ -0,0 +1,9 @@ | |||
| default-blocking-io-dispatcher { | |||
|
|
||
| /** | ||
| * | ||
| */ |
| public class App { | ||
|
|
||
| public static void main(String[] args) { | ||
| ConfigurableApplicationContext context = SpringApplication.run(App.class, args); |
There was a problem hiding this comment.
Please don't format the code using tabs -- follow the same style as other examples do, which is 2 spaces
|
|
||
| <dependency> | ||
| <groupId>com.typesafe.akka</groupId> | ||
| <artifactId>akka-actor_2.11</artifactId> |
There was a problem hiding this comment.
Scala 2.12 instead, extract it into a property as well?
| <dependency> | ||
| <groupId>com.typesafe.akka</groupId> | ||
| <artifactId>akka-slf4j_2.11</artifactId> | ||
| <version>2.5.3</version> |
| default-blocking-io-dispatcher { | ||
| type = "Dispatcher" | ||
| executor = "thread-pool-executor" | ||
| throughput = 1 |
| <artifactId>akka-slf4j_2.11</artifactId> | ||
| <version>2.5.3</version> | ||
| </dependency> | ||
| <dependency> |
There was a problem hiding this comment.
Why not also show the alpakka reactive-streams integration? https://akka.io/blog/news/2017/10/23/native-akka-streams-in-spring-web-and-boot-via-alpakka
There was a problem hiding this comment.
I'm currently learning Akka Streams for Kafka integration, I could add those in a few days :)
|
Not sure why the white space format is not being updated to github. Is this some kind of issue with github? Can't it recognize when the format is changed from tabs to white spaces? |
| @Component | ||
| public class SpringExtension implements Extension { | ||
|
|
||
| private ApplicationContext applicationContext; |
There was a problem hiding this comment.
Extensions are responsible to be thread safe, given that this field must be volatile or an AtomicReference else calls to the props methods (from different threads) may not see that it was initialized with any value
There was a problem hiding this comment.
Is making the Props methods Thread-safe with the "synchronized" keyword enough? Also, I'm reading from the docs that the thread safety is important in case multiple Actor Systems use the Extension.
"Since this Extension is a shared instance per ActorSystem we need to be threadsafe"
Does this mean that for a single actor system, the thread safety is not necessary? What about for cluster sharding?
There was a problem hiding this comment.
No, that is an incorrect interpretation of the docs.
An instance of an extension that is bound to a single actor system, the actor system ensure there is only one instance for the actor system, but the thread safety is not guaranteed (like with actors), in that sense it is just a normal object, and since the methods of the extension is called from multiple threads this means they must be thread safe.
Volatile or atomic reference is better than synchronized as they to don't cause any locking.
| public void printService(String string) { | ||
| System.out.println("Printed with bean: " + string); | ||
| } | ||
| } |
There was a problem hiding this comment.
Would a typical spring "Service" contain mutable state and be injected in multiple actors? In that case please add a comment here that the service must be thread safe.
There was a problem hiding this comment.
It's not really a state because it's not an attribute. The service just "processes" the input string, in this case by printing it to console. A very simple example to show that the Autowired annotation works.
There was a problem hiding this comment.
Yep, I got that the sample is stateless, was more thinking of how it would be in a real(tm) spring app. People tend to copy paste these sample and build whole applications so some friendly comment-nudging in the right direction is often good.
| } | ||
|
|
||
| public static class SpringExt implements Extension { | ||
| private volatile ApplicationContext applicationContext; |
There was a problem hiding this comment.
This is the closest I found to the documentation. The only difference is that the Extension class is inside the AbstractExtensionId class.
| @Bean(destroyMethod = "terminate") | ||
| public ActorSystem actorSystem() { | ||
| ActorSystem actorSystem = ActorSystem.create("akka-system", akkaConfiguration()); | ||
| SpringExtension.SpringExtProvider.get(actorSystem).initialize(applicationContext); |
There was a problem hiding this comment.
This means that there is a potential short time when the SpringExtProvider is not yet initialised and trying to create an actor using it would fail with a NPE.
This could be made completely safe byt passing the applicationContext to the extension as a akka.actor.setup.Setup instead, and pick it up from the actor system in createExtension. That way the application context field of the extension could also be final instead of having to be volatile.
There was a problem hiding this comment.
Hm, at this point I am a little lost. I'm very new to the way Akka works. I understand that the ActorSystem is initialized before the Extension, thus, the ActorSystem could try to create an Actor using the Extension just to find that the Extension´s ApplicationContext attribute is still null.
If so, I'm lost here because the initialization of the ActorSystem would depend on the initialization of the Extension's ApplicationContext, and viceversa.
Could I get a little extra help to further improve the PR?
There was a problem hiding this comment.
The Setup API allows you to define and pass objects into the actorsystem that is then available through ActorSystem.settings().setup(). Create your own Setup subclass which takes the application context to its constructor and keeps it as a final field. Something like this:
class SpringSetup extends Setup { ... }
// where you create the actor system
ActorSystem system = ActorSystem.create("system-name",
ActorSystemSetup.create(new SpringSetup(applicationContext)));
// in the extension
ApplicationContext ctx = system.settings().setup().get(SpringSetup.class)
.orElseThrow(new RuntimeException("ActorSystem must be created with a SpringSetup"))
.getApplicationContext
|
@AndresPineros I think this could be a very useful PR, would you be interested in having a look at the remaining review comments? |
|
Closing this due to inactivity, feel free to reopen if you want to resume work. |
No description provided.