RethinkDbPublisher, define timeout

[cdsap]

When there are problems of connectivity with RethinkDbPublisher executions hangs with:

[RethinkDbPublisher]: ================
[RethinkDbPublisher]: RethinkDbPublisher
[RethinkDbPublisher]: publishBuildMetrics: true
[RethinkDbPublisher]: publishTaskMetrics: true
[RethinkDbPublisher]: ================
[Talaiot]: Shutting down executor. Not yet. Still waiting for termination
[Talaiot]: Shutting down executor. Not yet. Still waiting for termination
[Talaiot]: Shutting down executor. Not yet. Still waiting for termination
[Talaiot]: Shutting down executor. Not yet. Still waiting for termination
[Talaiot]: Shutting down executor. Not yet. Still waiting for termination
[Talaiot]: Shutting down executor. Not yet. Still waiting for termination

Check timeout options in RethinkDbPublisher

[mokkun]

Maybe we could use an ExecutorService and await the execution with a timeout?

class RethinkDbPublisher(
...
    /**
     * Executor to schedule a task in Background
     */
    private val executor: ExecutorService
): Publisher {
...
    override fun publish(report: ExecutionReport) {
        ...
        try {
            // Executes the task.
            executor.execute { publishData(report) }
            // Don't wait for new tasks, execute only what we have.
            executor.shutdown()
            // Give 30 seconds for all pending tasks to finish.
            executor.awaitTermination(30, TimeUnit.SECONDS)
        } catch (e: Exception) {
            logTracker.error("RethinkDbPublisher- Error executing the Runnable: ${e.message}")
        }
    }

    private fun publishData(report: ExecutionReport) {
        logTracker.log(TAG, "================")
        logTracker.log(TAG, "RethinkDbPublisher")
        logTracker.log(TAG, "publishBuildMetrics: ${rethinkDbPublisherConfiguration.publishBuildMetrics}")
        logTracker.log(TAG, "publishTaskMetrics: ${rethinkDbPublisherConfiguration.publishTaskMetrics}")
        logTracker.log(TAG, "================")
        ...
    }
...
}

The timeout could be set as a constructor property in the RethinkDbPublisher class, to allow customization.

[mokkun]

Also, this issue may also happen with other executors like ElasticSearchPublisher and even InfluxDbPublisher? InfluxDbPublisher has a timeout for the connection, but not for the Executor.

It would be interesting to create a common implementation for those Publishers that need to execute network I/O (or any I/O for that matter).

[cdsap]

Hi @mokkun
First, yep, the case using awaitTermination covers the case for the RethinkDbPublisher. It should be applied for the other publishers, but in case of InfluxDb it's defined in the api of the influx client.

This bring to your second point about the "Network" implementation for the interface Publisher. I'm absolutely agree. Talaiot needs to implement a consistent network layer.
Before entering in details I have some notes:

• Some publishers are using external dependencies to abstract the network logic,our new network implementation should cover cases when we have to use external apis, for example:
  -- RethinkDbPublisher --> com.rethinkdb:rethinkdb-driver:
  -- ElasticDbPublisher --> org.elasticsearch.client:elasticsearch-rest-high-level-client:7.3.0
  -- InfluxDb --> org.influxdb:influxdb-java:
• What do you think if for the threading model use Coroutines? First version of Talaiot was under 4.x Gradle Version. When we were targeting lower AGP versions, there were problems integrating Coroutines because the embedded Kotlin Compiler. Now is not a problem, should we move to coroutines in the new implementation?

Do you have more notes? @MyDogTom do you have any suggestions/notes for this rework of the Network Layer?
Once we have the notes/suggestion collected we can create the issues/tasks and start working in this rework.

So, thank you very much for this proposals, I really appreciate it.

[MyDogTom]

Sorry I'm afraid I don't have enough knowledge to give a valuable input here. I'll definitely take a closer look at the implementation to learn more.
btw, time to time I also see

[Talaiot]: Shutting down executor. Not yet. Still waiting for termination

with InfluxDbPublisher, but only one occurrence per build.

[mokkun]

Some publishers are using external dependencies to abstract the network logic, our new network implementation should cover cases when we have to use external APIs...

One idea would be to provide these publishers as external plugins to Talaiot. These publishers are very specific IMO, and many projects won't use them.

The main module of Talaiot could provide the base APIs for those plugins, but the network logic and dependencies would be contained in them, instead of in the main module.

What do you think if for the threading model use Coroutines? First version of Talaiot was under 4.x Gradle Version. When we were targeting lower AGP versions, there were problems integrating Coroutines because the embedded Kotlin Compiler. Now is not a problem, should we move to coroutines in the new implementation?

Sounds good to me. 👍

[cdsap]

Hi, it sounds good the detachment of the publisher by offering a new dependency.

Given the scope of the changes, I've created a new milestone to publish the 2.0 release version. At the moment I included two new issues:
#164
#165
I'm going to take the Detach the main Plugin dependencies by Publisher.
Feel free to work in the #165. I guess if I finish before the #164, pulling changes for a new implementation wouldn't be a big deal.
Any of you have more suggestions for the 2.0 release?