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

Modify PackateQueue to let auth request go first and other request go last #52

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

XadillaX
Copy link

@XadillaX XadillaX commented Oct 24, 2016

See code below:

const client = ...;

client.addAuthInfo(...);
client.getData(...);

client.connect();

getData's command will be at the first position of the queue array and auth command will be the second.

Because getData will push to queue immediately but addAuthInfo will push after connected.

ConnectionManager.prototype.addAuthInfo = function(scheme, auth) {
    // ...
    switch(this.state) {
    // ...
    case STATES.DISCONNECTED:
        return;
    }
};

ConnectionManager.prototype.onSocketConnected = function() {
    if(this. credentials.length > 0) {
        // ...
        this.queue.push(...);
    }
};

Client.prototype.getData = function(path, watcher, callback) {
    // ...
    self.connectionManager.queue(...);
};

So I modify PacketQueue.prototype.push to let auth request go the first.

@XadillaX
Copy link
Author

Anyone to review this?

@alexguan
Copy link
Owner

I will take a look this weekend

@XadillaX
Copy link
Author

So how's the result?

@alexguan
Copy link
Owner

In what scenario you will call getData before the connection is established?

@XadillaX
Copy link
Author

XadillaX commented Aug 28, 2017

@alexguan Sorry but I forgot the reason because it takes a long time. Maybe reconnecting?

@subashcs
Copy link

subashcs commented Feb 16, 2024

This PR looks good. This is a much-needed PR for situations when there are multiple watchers receiving data and suddenly authentication fails due to any reason. To reinitiate the authentication zookeeper initiates another auth request. Since the auth request is not prioritized in the queue, the other requests initiated by the watchers fail until the authentication request passes.

@alexguan
Copy link
Owner

Sorry for the long delay, can someone bump up the version and changelog like this PR and I will merge it.

@subashcs
Copy link

@alexguan Are we supposed to create a new PR for the version bump?

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

Successfully merging this pull request may close these issues.

4 participants