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

#387 temporary set timeout #405

Closed
wants to merge 4 commits into from
Closed

#387 temporary set timeout #405

wants to merge 4 commits into from

Conversation

vchangpengfei
Copy link
Contributor

@vchangpengfei vchangpengfei commented Dec 17, 2021

#387 temporary set timeout
#401 set default check connection

@Nicole00
Copy link
Contributor

Nicole00 commented Dec 20, 2021

Thanks for your contribution. I have two questions:

  1. Why don't set timeout 0 to avoid the timout issue?
  2. how to test the default check for connection?

@vchangpengfei
Copy link
Contributor Author

vchangpengfei commented Dec 21, 2021

  1. Why don't set timeout 0 to avoid the timout issue?
    Sorry didn t understand what you mean, here is temporary set, if set to 0 it should be the same as Integer.MAX_VALUE ,should be ok ,just I set a condition here
if (timeout > 0) {
    transport.setTimeout(timeout);
}
  1. how to test the default check for connection?

This problem we have already validated in the production environment and will automatically disappears when we catch this excepiton and release the connection

image

image

connection is managed by ConnObjectPool,conncetion·s created、borrow、return will checks if is available,

    @Override
    public boolean validateObject(PooledObject<SyncConnection> p) {
        if (p.getObject() == null) {
            return false;
        }
        if (!p.getObject().ping()) {
            p.getObject().close();
            return false;
        }
        return true;
    }

@Nicole00
Copy link
Contributor

I mean since the 0 timeout can avoid the timeout issue, why need a extra temporary timeout for each execution?

@vchangpengfei
Copy link
Contributor Author

I mean since the 0 timeout can avoid the timeout issue, why need a extra temporary timeout for each execution?

Time out time Some scenes have to be set, or it may drag down the called application, causing an avalanche
Not set every time because the session can be reused and temporary settings are only required when the session is created

@vchangpengfei
Copy link
Contributor Author

vchangpengfei commented Dec 21, 2021

I mean since the 0 timeout can avoid the timeout issue, why need a extra temporary timeout for each execution?

As a simple example, the data in the nebula is weakly dependent on the nebula, but the nebula response time in some case, the response time slows down
If the timeout is not set, the response time of the application will become longer, so that the throughput of the application will reduce, resulting in a large number of timeout upstream, resulting in an avalanche

@vchangpengfei
Copy link
Contributor Author

I mean since the 0 timeout can avoid the timeout issue, why need a extra temporary timeout for each execution?

I wonder if I express it clear

@Nicole00
Copy link
Contributor

Nicole00 commented Dec 23, 2021

I mean since the 0 timeout can avoid the timeout issue, why need a extra temporary timeout for each execution?

I wonder if I express it clear

Yeah, I got you. But why don't set a monitor to make it timeout in the business layer.

@vchangpengfei
Copy link
Contributor Author

I mean since the 0 timeout can avoid the timeout issue, why need a extra temporary timeout for each execution?

I wonder if I express it clear

Yeah, I got you. But why don't set a monitor to make it timeout in the business layer.

you kan see this issues

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.

2 participants