Thanks for your reply Evegeny. I added this change because I intend to use it regardless. I was hoping I could answer some of Marc's concerns by having him review the change, but this is the build we'll be using internally.
The shutdown hook with the daemon timer should be as stable as it was with the daemon worker thread in terms of a shutdown hook. I wasn't aware that any of the tests had failed but I'll take a look. I said in my request that I'd be happy to add tests and documentation pending feedback, but based on what has already been said I wasn't optimistic this would get picked up. If you consider the case where the server fails during data transfer, are you any worse off if the client tries to reconnect 3 seconds later? The server has already failed and the data is already (potentially) lost, adding retry didn't cause that. You could argue that retry makes it less obvious that only some of the data was uploaded. I'd be open to logging the exception before scheduling the reconnect if that makes any difference to you. I can also add synchronization to shared member changes if that would satisfy your concerns, but I think I have my answer. Sincerely thank you for your feedback and for maintaining this project. Best regards Nathan On Tue, Oct 20, 2020 at 3:53 PM Evgeny Mandrikov <[email protected]> wrote: > > On Tue, Oct 20, 2020 at 6:42 PM Nathan Wray <[email protected]> wrote: > >> The changes here simply allow it to recover at no real cost in complexity. > > > Here is what is usually and maybe even here underestimated / not > considered - every additional feature has at least costs of future answers > to user questions about it ;) > > IMO one of the main concerns is about > > On Tue, Sep 29, 2020 at 6:23 AM Marc Hoffmann <[email protected]> > wrote: > >> data loss >> > > On Thu, Oct 15, 2020 at 7:20 PM Nathan Wray <[email protected]> wrote: > >> ignore any data that happened to be in transit during a failure >> > > while for your use-case data loss might be acceptable, this is not > necessarily the case for others and might be really hard to diagnose for us > when some users will face this. > > And I'm not sure it was addressed because > > On Thu, Oct 15, 2020 at 8:00 PM Marc Hoffmann <[email protected]> > wrote: > >> I’m still not convinced it is a good idea to add distributed/resilient to >> the coverage agent. There are simply too many failure scenarios to test and >> get right. Especially in the case of JVM shutdown you don’t want to have >> wait/retry loops. >> > > Please trust our experience - JVM shutdown hooks are not easy to get done > right, especially in the case of multiple threads, thread groups, timers. > For example to me not obvious why your changes do not add any > synchronization, aren't timer thread modifies "connection" field that > shutown thread reads? > And I don't see any new tests, while existing are failing. > > Moreover, I don't understand why you're asking to review changes, despite > the fact that in all responses project maintainers aren't in favor of this > idea in general and suggest using JaCoCo APIs instead. > Not every feature has to be in core, especially if it can be implemented > using already provided APIs - why not start developing your idea as a > separate project without modifications in JaCoCo, so that you'll be able to > learn by feedback from users about the robustness of implementation and > approach in general, we'll be happy to mention it in > https://www.jacoco.org/jacoco/trunk/doc/integrations.html > > Regards, > Evgeny > > -- > You received this message because you are subscribed to a topic in the > Google Groups "JaCoCo and EclEmma Users" group. > To unsubscribe from this topic, visit > https://groups.google.com/d/topic/jacoco/QsyvL7bUDng/unsubscribe. > To unsubscribe from this group and all its topics, send an email to > [email protected]. > To view this discussion on the web visit > https://groups.google.com/d/msgid/jacoco/CAEPFu6_BmBGuADDacq4LENRywTabOWscRuWr61bdnDnY2xoB2A%40mail.gmail.com > <https://groups.google.com/d/msgid/jacoco/CAEPFu6_BmBGuADDacq4LENRywTabOWscRuWr61bdnDnY2xoB2A%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > -- You received this message because you are subscribed to the Google Groups "JaCoCo and EclEmma Users" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/jacoco/CANX5QUhi54DW0JnkSfVikS_MNuOF%3DP08XpmVA0SERy3qB02CCA%40mail.gmail.com.
