pgjdbc/pgjdbc GitHub issues and pull requests (mirror)  
help / color / mirror / Atom feed
[pgjdbc/pgjdbc] issue #879: loginTimeout in java.sql.DriverManager never be used
14+ messages / 5 participants
[nested] [flat]

* [pgjdbc/pgjdbc] issue #879: loginTimeout in java.sql.DriverManager never be used
@ 2017-07-23 14:03 "Frankqsy (@Frankqsy)" <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Frankqsy (@Frankqsy) @ 2017-07-23 14:03 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

in the file:pgjdbc/src/main/java/org/postgresql/PGProperty.java,line 246.
```
 LOGIN_TIMEOUT("loginTimeout", "0",
      "Specify how long to wait for establishment of a database connection.");
```
the default value "0" will lead to the result that the value set in DriverManager never be used.because the code using the value as follows:
in the file:pgjdbc/pgjdbc/src/main/java/org/postgresql/Driver.java,line 633
```
  private static long timeout(Properties props) {
    String timeout = PGProperty.LOGIN_TIMEOUT.get(props);
    if (timeout != null) {
      try {
        return (long) (Float.parseFloat(timeout) * 1000);
      } catch (NumberFormatException e) {
        LOGGER.log(Level.WARNING, "Couldn't parse loginTimeout value: {0}", timeout);
      }
    }
    return (long) DriverManager.getLoginTimeout() * 1000;
  }
```
and in the method "get" of PGProperty,the code as follows:
in the file:pgjdbc/src/main/java/org/postgresql/PGProperty.java,line 475.
```
public String get(Properties properties) {
    return properties.getProperty(_name, _defaultValue);
  }
```
as above,the LOGIN_TIMEOUT will always get the default value of "0",the loginTimeout value set in the DriverManager will never be used.
if we want to set the loginTimeout, we must put it into properties,this will make confusion when we develop. 
The phenomenon will be that the login will hang if the network is error  although we have set the loginTimeout value in the DriverManager.

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [pgjdbc/pgjdbc] issue #879: loginTimeout in java.sql.DriverManager never be used
@ 2017-07-23 14:06 ` "davecramer (@davecramer)" <[email protected]>
  12 siblings, 0 replies; 14+ messages in thread

From: davecramer (@davecramer) @ 2017-07-23 14:06 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

Thanks for the report. Seems easy enough to fix. Did you have a PR ?

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [pgjdbc/pgjdbc] issue #879: loginTimeout in java.sql.DriverManager never be used
@ 2017-07-23 14:20 ` "Frankqsy (@Frankqsy)" <[email protected]>
  12 siblings, 0 replies; 14+ messages in thread

From: Frankqsy (@Frankqsy) @ 2017-07-23 14:20 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

no.I can fix later,or you can fix it~

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [pgjdbc/pgjdbc] issue #879: loginTimeout in java.sql.DriverManager never be used
@ 2017-08-03 02:47 ` "zemian (@zemian)" <[email protected]>
  12 siblings, 0 replies; 14+ messages in thread

From: zemian (@zemian) @ 2017-08-03 02:47 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

Hi, I want to help out and created a PR #900 for this. The first commit fixed the issue reported here. And then I saw that there is no reason to use Float.parseFloat to parse int value (I assume loginTimeout value should always be integer in secs), so I made 2nd commit to improve this as well. Let me know if that works for you.

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [pgjdbc/pgjdbc] issue #879: loginTimeout in java.sql.DriverManager never be used
@ 2017-08-03 03:07 ` "Frankqsy (@Frankqsy)" <[email protected]>
  12 siblings, 0 replies; 14+ messages in thread

From: Frankqsy (@Frankqsy) @ 2017-08-03 03:07 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

why don't you just set the default value to null, it's more easy. Just as follows:
```
LOGIN_TIMEOUT("loginTimeout", null,
      "Specify how long to wait for establishment of a database connection.");
```
I guess the original author has his concern to make the value precise to milliseconds.
So i suggest that you don't change its logical code.

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [pgjdbc/pgjdbc] issue #879: loginTimeout in java.sql.DriverManager never be used
@ 2017-08-03 04:54 ` "zemian (@zemian)" <[email protected]>
  12 siblings, 0 replies; 14+ messages in thread

From: zemian (@zemian) @ 2017-08-03 04:54 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

Because having NULL as default for int param seems odd.

If milliseconds were intented then it should take millis as long unit value Instead. I think it's odd to take float as parameter and call it secs.

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [pgjdbc/pgjdbc] issue #879: loginTimeout in java.sql.DriverManager never be used
@ 2017-08-03 05:17 ` "Frankqsy (@Frankqsy)" <[email protected]>
  12 siblings, 0 replies; 14+ messages in thread

From: Frankqsy (@Frankqsy) @ 2017-08-03 05:17 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

How about the case that the user set LOGIN_TIMEOUT to "0" while the value in DriverManager is not 0?
Which one do you choose?
And i suggest you to use Long instead of Integer. 

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [pgjdbc/pgjdbc] issue #879: loginTimeout in java.sql.DriverManager never be used
@ 2017-08-03 13:08 ` "zemian (@zemian)" <[email protected]>
  12 siblings, 0 replies; 14+ messages in thread

From: zemian (@zemian) @ 2017-08-03 13:08 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

The current fix will use PG conn property if it's NOT zero, else it will use DriverManager. So your case will use second one. The DriverManager also default to zero if you don't set anything.

Note that the DriverMangaer.loginTimeout is documented as secs and is using "int". The PG conn properly "loginTimeout" is also documented as in "secs" as well, and a max java "int" value can hold about 68 years! Is Long really needed for such timeout?

The only concern I have myself, as you already pointed out, is that changing from Float.parseFloat to Integer.parseInt might break existing user config if they already started using decimal number values for loginTime (Well, actually they will get a WARNING msg b/c bad parsing, and then default to DriverManager.loginTimeout() instead). But as I argued, it will make PG conn properly consistent and easier to document from API perspective. So it will be a decision for the PG committers to decide whether they want this improvement or not.

I am happy to make any modification you guys need. Just let me know what's more reasonable.


^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [pgjdbc/pgjdbc] issue #879: loginTimeout in java.sql.DriverManager never be used
@ 2020-11-13 07:05 ` "gung-rgb (@gung-rgb)" <[email protected]>
  12 siblings, 0 replies; 14+ messages in thread

From: gung-rgb (@gung-rgb) @ 2020-11-13 07:05 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

Why this issue is still open? and The latest version is still has the same problem?

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [pgjdbc/pgjdbc] issue #879: loginTimeout in java.sql.DriverManager never be used
@ 2020-11-14 00:13 ` "davecramer (@davecramer)" <[email protected]>
  12 siblings, 0 replies; 14+ messages in thread

From: davecramer (@davecramer) @ 2020-11-14 00:13 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

@gongmingbin because we asked the author of the PR to make changes and this has not been done. If you wish you can fix up the PR so we can commit it

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [pgjdbc/pgjdbc] issue #879: loginTimeout in java.sql.DriverManager never be used
@ 2020-11-14 18:18 ` "zemian (@zemian)" <[email protected]>
  12 siblings, 0 replies; 14+ messages in thread

From: zemian (@zemian) @ 2020-11-14 18:18 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

Hi Dave, I wasn't aware that there were pending items from me to change further. I think it's just waiting for committer to review and approve the PR I submitted. If there is anything you need, please let me know and I can help put it in.

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [pgjdbc/pgjdbc] issue #879: loginTimeout in java.sql.DriverManager never be used
@ 2020-11-17 19:08 ` "davecramer (@davecramer)" <[email protected]>
  12 siblings, 0 replies; 14+ messages in thread

From: davecramer (@davecramer) @ 2020-11-17 19:08 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

@zemian see https://github.com/pgjdbc/pgjdbc/pull/900/files for requests for changes

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [pgjdbc/pgjdbc] issue #879: loginTimeout in java.sql.DriverManager never be used
@ 2020-11-18 15:25 ` "zemian (@zemian)" <[email protected]>
  12 siblings, 0 replies; 14+ messages in thread

From: zemian (@zemian) @ 2020-11-18 15:25 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

Hi Dave, I think I did address all the requests for changes. See all the
replies I made on the "Conversation" tab side. The last reply I made was
this: https://github.com/pgjdbc/pgjdbc/pull/900#issuecomment-341902369
regarding test assertion review comments. I thought I did my best and was
just waiting for approval. :)

I have now moved on to another project and I don't have much free time. So
if something quick, I will be happy to help, otherwise you guys might need
to pick up the rest of change.


On Tue, Nov 17, 2020 at 2:08 PM Dave Cramer <[email protected]>
wrote:

> @zemian <https://github.com/zemian; see
> https://github.com/pgjdbc/pgjdbc/pull/900/files for requests for changes
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pgjdbc/pgjdbc/issues/879#issuecomment-729139520;, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AFJ2JECY7CXFCW55B6352YLSQLC2DANCNFSM4DUB3BZQ;
> .
>


^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: [pgjdbc/pgjdbc] issue #879: loginTimeout in java.sql.DriverManager never be used
@ 2025-10-13 19:59 ` "jrobe (@jrobe)" <[email protected]>
  12 siblings, 0 replies; 14+ messages in thread

From: jrobe (@jrobe) @ 2025-10-13 19:59 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

Hi, I come from 5 years in the future following a 1 hour long production outage in my services due to this bug... (I was using Driver-mode Hikari and loginTimeout settings from it weren't being respected). Any chance we can get some movement here?

^ permalink  raw  reply  [nested|flat] 14+ messages in thread


end of thread, other threads:[~2025-10-13 19:59 UTC | newest]

Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-07-23 14:03 [pgjdbc/pgjdbc] issue #879: loginTimeout in java.sql.DriverManager never be used "Frankqsy (@Frankqsy)" <[email protected]>
2017-07-23 14:06 ` "davecramer (@davecramer)" <[email protected]>
2017-07-23 14:20 ` "Frankqsy (@Frankqsy)" <[email protected]>
2017-08-03 02:47 ` "zemian (@zemian)" <[email protected]>
2017-08-03 03:07 ` "Frankqsy (@Frankqsy)" <[email protected]>
2017-08-03 04:54 ` "zemian (@zemian)" <[email protected]>
2017-08-03 05:17 ` "Frankqsy (@Frankqsy)" <[email protected]>
2017-08-03 13:08 ` "zemian (@zemian)" <[email protected]>
2020-11-13 07:05 ` "gung-rgb (@gung-rgb)" <[email protected]>
2020-11-14 00:13 ` "davecramer (@davecramer)" <[email protected]>
2020-11-14 18:18 ` "zemian (@zemian)" <[email protected]>
2020-11-17 19:08 ` "davecramer (@davecramer)" <[email protected]>
2020-11-18 15:25 ` "zemian (@zemian)" <[email protected]>
2025-10-13 19:59 ` "jrobe (@jrobe)" <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox