Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Asynchronous initial connection #406

Closed
agcom opened this issue Feb 9, 2021 · 7 comments
Closed

Asynchronous initial connection #406

agcom opened this issue Feb 9, 2021 · 7 comments

Comments

@agcom
Copy link

agcom commented Feb 9, 2021

Current experimental Nats.connectAsynchronously function is so bad that I don't have enough time to convince you 😄.

However, the problem arises from NatsImpl.createConnection which won't return until the initial connection is made. And this is because of the library design which distinguishes the initial connection and reconnect attempts anyway (the reconnectOnConnect boolean is not effective).

Short term proposal

...
public class Nats {
    ...
    // Named 'create', because it returns immediately.
    public static Connection create(final Options options) {
        return NatsImpl.createAsyncConnection(options);
    }
    ...
}
...
public class NatsImpl {
    ...
    public static Connection createAsyncConnection(final Options options)  {
        final NatsConnection conn = new NatsConnection(options);
        // Somehow trigger the client to attempt (re)connecting;
        // The following solution works, but it would be more expressive if there was a trigger function which would schedule the connection attempt(s) in its executor. For example, conn.init().
        final Thread t = new Thread(() -> {
            try {
                conn.connect(true);
            } catch (final Exception ex) {
                if (options.getErrorListener() != null) {
                    options.getErrorListener().exceptionOccurred(conn, ex);
                }
            }
        });
        t.start();
        
        return conn;
    }
    ...
}

Long term proposal

MongoDB reactive Java driver is a good example. It completely acts as a black box and doesn't distinguish between initial connection and reconnect attempts. It also uses reactive streams APIs (project reactor and RXJava) instead of rewriting the wheel.

@scottf scottf self-assigned this Mar 8, 2021
@scottf scottf changed the title Asynchronous connection Asynchronous initial connection Apr 30, 2021
@scottf scottf removed their assignment Apr 30, 2021
@brimworks
Copy link
Collaborator

@agcom although I like the idea (go all async), I think that would essentially be rewrite, don't you think? ...and if such a substantial rewrite was undertaken, would it even have a chance of getting adopted by nats.io as a replacement for the current supported client? FWIW, I have found a few "undocumented" features of NATS such as the lovely 503 "NATS/1.0" status code as part of the HMSG, so at this point undertaking a rewrite sounds pretty overwhelming to me.

@scottf do you have any thoughts on this?

...or perhaps I misunderstand what you are trying to accomplish @agcom? Is your concern more about the current implementation of reconnects? To be honest, I like the fact that initial connects are treated specially so that if the first connect fails I get immediate feedback rather than having bad credentials only to have some background thread print a stack trace each time it tries to auto reconnect.

@scottf
Copy link
Contributor

scottf commented Aug 12, 2021

The core of this issue is

the library design which distinguishes the initial connection and reconnect attempts

and that is likely not going to change any time soon.

@brimworks
Copy link
Collaborator

Maybe we should close this as won't fix then?

@agcom
Copy link
Author

agcom commented Aug 12, 2021

@brimworks Yes. What I proposed as "Long term proposal" is essentially a rewrite; going completely non-blocking and built-in integration with richer APIs (Kotlinx coroutines, RXJava, project reactor). No idea about nats.io but such a rewrite would be adopted by the community for sure.

...I like the fact that initial connects are treated specially so that if the first connect fails I get immediate feedback...

You could do the same using async APIs (maybe with a little twist); bonus point is that you won't pin an OS thread while doing so.

...only to have some background thread print a stack trace each time it tries to auto reconnect...

That would be a terrible rewrite (not being able to monitor your client's status).

@brimworks
Copy link
Collaborator

It looks like there are several issues which involve the idea of doing a rewrite using non-blocking IO:

#347
#317

Maybe we can close all these issues into a single issue? The title of #317 seems more appropriate, so I'd propose closing this ticket and #347.

Is that okay?

@scottf
Copy link
Contributor

scottf commented Aug 12, 2021

@brimworks I like that idea of combining them into one. RxJava is on our long term list of provide some sort of integration.

@brimworks
Copy link
Collaborator

Tracking this need here: #317

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants