makes the various timeouts configurable on the connection -- connect,…

[bbeaudreault]

… query, transaction

We have a need to configure these settings. Also, we have a need to disable the timeouts in some cases -- such as database migrations which may take a long time. Therefore, if the configured timeout is set to 0 it is ignored createContext

[harshit-gangal]

Thanks for thinking about this.
AFAIK,
Clients can any control queryTimeout.

Transaction timeout is configured on VtTablets.
Connection timeout is not properly.
Only change we can do in clients is about queryTimeout.

---

Reviewed 4 of 7 files at r1.
Review status: 4 of 7 files reviewed at latest revision, 1 unresolved discussion.

---

java/jdbc/src/main/java/io/vitess/jdbc/ConnectionProperties.java, line 210 at r1 (raw file):

        true);

    private LongConnectionProperty queryTimeoutMillis = new LongConnectionProperty(

I think we can remove millis from the property and just mention in the property description.

---

Comments from Reviewable

[bbeaudreault]

@harshit-gangal I renamed them as requested, removing the millis suffix.

I see your point that client cannot modify the actual connection and transaction timeouts in vttablet. But as you can see in the diff, there were 3 timeouts used for a) creating the grpc connection, b) calling commit/rollback, c) other queries.

I do not want any hardcoded timeouts, as we had before. So that is why there are 3 new properties.

[bbeaudreault]

I'm away for the next week if any further comments come up. But hopefully this should be good to go.

[harshit-gangal]

I meant, we can just have queryTimeout property for now and use it everywhere, till we do not support other property end to end.

---

Reviewed 3 of 7 files at r1, 5 of 5 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

java/jdbc/src/main/java/io/vitess/util/CommonUtils.java, line 36 at r1 (raw file):

     * @return
     */
    public static Context createContext(String username, long connectionTimeout) {

we can rename connectionTimeout to timeout. As it is only query execution timeout.

---

Comments from Reviewable

[bbeaudreault]

@harshit-gangal ok sounds good. I'll unify everything on a single queryTimeout config. I'll try to do that tomorrow morning so we can hopefully merge before I leave for the week.

[bbeaudreault]

@harshit-gangal ok I renamed to timeout and unified the properties on just the 1

[harshit-gangal]

---

Reviewed 8 of 8 files at r3.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

Comments from Reviewable

[sougou]

LGTM