Issue 84: Channel not shutdown correctly in TurbineHeatSensor sample #110
Conversation
| @@ -142,6 +142,7 @@ public static void main(String[] args) throws Exception { | |||
| if ( !onlyWrite ) { | |||
| consumeStats.printTotal(); | |||
| } | |||
| clientFactory.close(); | |||
| // ZipKinTracer.getTracer().close(); | |||
There was a problem hiding this comment.
Remove this commented line in a new PR, please.
There was a problem hiding this comment.
Ok, I will do that.
There was a problem hiding this comment.
This (and other issues of this app) are reported in #119.
| @@ -142,6 +142,7 @@ public static void main(String[] args) throws Exception { | |||
| if ( !onlyWrite ) { | |||
| consumeStats.printTotal(); | |||
| } | |||
| clientFactory.close(); | |||
There was a problem hiding this comment.
This is a bit confusing as we are closing in the main method and in the runLoop. Is the contract that the runnable is supposed to close the factory or is this something that needs to be done by the caller? Let's be consistent about how we are closing it.
There was a problem hiding this comment.
To follow a consistent pattern without changing much the current code: i) now there is only one client factory that is created and closed within the main method. ii) Writers and readers take as input that factory to instantiate an EventStream[Writer/Reader], iii) writers and readers are in charge of closing their own EventStream[Writer/Reader]. In any case, a future PR for issue #119 will be devoted to improve the code of this application, so we can find better ways of managing resources (out of the scope of this PR).
…he one passed by parameter. Signed-off-by: Raúl Gracia <[email protected]>
Signed-off-by: Raúl Gracia <[email protected]>
Signed-off-by: Raúl Gracia <[email protected]>
Signed-off-by: Raúl Gracia <[email protected]>
RaulGracia
force-pushed
the
issue-84-channel-not-shutdown-correctly
branch
from
58e6282 to
1fe01de
Compare
Signed-off-by: Raúl Gracia <[email protected]>
Signed-off-by: Raúl Gracia <[email protected]>
Signed-off-by: Raúl Gracia <[email protected]>
| executor.execute(reader); | ||
| } | ||
| /* Create producerCount number of threads to simulate sensors. */ | ||
| Instant startEventTime = Instant.EPOCH.plus(8, ChronoUnit.HOURS); // sunrise | ||
| for (int i = 0; i < producerCount; i++) { | ||
| URI controllerUri = new URI(TurbineHeatSensor.controllerUri); | ||
| ClientFactory factory = ClientFactory.withScope(scopeName, controllerUri); |
There was a problem hiding this comment.
Is ClientFactory thread safe?
There was a problem hiding this comment.
The ClientFactory object is only used in the constructor of SensorReader and TemperatureSensors to create writers/readers, and they do not hold any reference to the factory object. This yields that only the main thread is actually operating on the ClientFactory (no concurrency), while the multiple running threads operate each one on its own EventStreamReader/Writer.
…110) * Fixes in TurbineHeatSensor.java the use of client factories while creating writers. Note that this PR specifically targets to avoid java.lang.RuntimeException: ManagedChannel allocation site exception to be thrown (e.g., the app may throw other exceptions that will be addressed separately). Signed-off-by: Raúl Gracia <[email protected]>
Change log description
Minor fix in
TurbineHeatSensor.javaregarding the use of client factories while creating writers. Note that this PR specifically targets to avoidjava.lang.RuntimeException: ManagedChannel allocation siteexception to be thrown (e.g., the app may throw other exceptions that will be addressed separately).Purpose of the change
Fixes #84.
What the code does
The constructor of
TemperatureSensorsreceives by parameter a client factory to create event writers. However, it was creating writers via a global static client factory, which was in turn shared to create readers as well. Creating readers/writers carelessly, jointly with the fact that resources are not correctly closed, could be the cause for the closing channel exception to be thrown. In this fix, we replaced the use of the global static client factory by the one passed by parameter to create writers. While this was enough to avoid the exception to occur, we also decided to close both client factories and writers for the sake of correctness.How to verify it
Execute the
turbineHeatSensorapplication and verify that the exception mentioned in the issue does not appear in the console.