RE: Timeout parameters

2019-04-07 Thread Nagaura, Ryohei
Hi, Michael-san.

> From: Michael Paquier [mailto:mich...@paquier.xyz]
> I have just committed the GUC and libpq portion for TCP_USER_TIMEOUT after a
> last lookup, and I have cleaned up a couple of places.  It is a bit 
> disappointing
> to see the option supported on Linux, but not on Windows, Solaris and BSD as
> far as I can see.  Still that's useful in itself for more control of the TCP
> connection.  Hopefully I forgot nobody in the review credits.
I confirmed your commitment. Thank you.

> I am marking the CF entry as committed.  In the future, it would be better to
> not propose multiple concepts on the same thread, and if the socket_timeout
> business is resubmitted, I would suggest a completely new CF entry, and a new
> thread.
Thank you for your suggestion.
I think it would be better too.

Best regards,
-
Ryohei Nagaura







RE: Timeout parameters

2019-04-04 Thread Nagaura, Ryohei
Hello.

> From: Jamison, Kirk 
> As for socket_timeout, I agree that this can be discussed further.
Yes, probably.

> From: Fabien COELHO 
> * About socket_timeout v12 patch, I'm not sure there is a consensus.
I think so too. I just made the patch being able to be committed anytime.

Finally, I rebased all the patches because they have come not to be applied to 
the updated PG.
Unlike before, note that the current socket_timeout patch premiss to be 
TCP_interface patch.

Best regards,
-
Ryohei Nagaura



TCP_backend_v20.patch
Description: TCP_backend_v20.patch


TCP_interface_v20.patch
Description: TCP_interface_v20.patch


socket_timeout_v14.patch
Description: socket_timeout_v14.patch


RE: Timeout parameters

2019-03-31 Thread Nagaura, Ryohei
Hello, Fabien-san.

> From: Fabien COELHO [mailto:coe...@cri.ensmp.fr]
> I have further remarks after Kirk-san extensive review on these patches.
Yes, I'm welcome.
Thank you very much for your review.


> * About TCP interface v18.
> For homogeneity with the surrounding cases, ISTM that "TCP_user_timeout"
> should be ""TCP-user-timeout".
Oops, it would be better. Thanks.
Note that I changed to "TCP-User-Timeout" not "TCP-user-timeout."
Also, I deleted this
+   /* TCP USER TIMEOUT */


> * About TCP backend v19 patch
> I still disagree with "on other systems, it must be zero.": I do not see why
> "it must be zero", I think that the parameter should simply be ignored if it
> does not apply or is not implemented on a platform?
No, please see the below with "src/backend/libpq/pqcomm.c".
I made consistent with Keepalives documentation and implementation.
i,e,. if the setting value is non-zero on a system that does not support 
these parameters, an "not supported" error is output.
Therefore, "on other systems, it must be zero" in doc.
Also, the error message "not supported" is needed because if an error is not 
output when you set that parameter on such a system, it seems to be available 
in spite of not available.
In my patch, this situation corresponds to line 114.

> If there are consistency constraint with other timeout parameters, probably 
> the
> documentation should mention it?
As I said above.


> * About socket_timeout v12 patch, I'm not sure there is a consensus.
> I still think that there should be an attempt at cancelling before
> severing.
When some accidents hits the server, infinite wait of the user may occur.
This feature is a way to get around it.
It is not intended to reduce server load.
Since sending a cancellation request will cause the user to wait[1], I do not 
think that it follows the intention.
Do you think that you still need to send a cancellation request?

> Robert pointed out that it is not a timeout wrt the query, but this is not
> clearly explained in the documentation nor the comments.
Sorry, I couldn't recognize your intension.
Is it that you think the description "this is not a query timeout" is needed?

> The doc says that
> it is the time for socket read/write operations, but it is somehow the
> time between messages, some of which may not be linked to read/write
> operations. I feel that the documentation is not very precise about what
> it really does.
What socket_timeout really does is a timeout by poll().
poll() monitors a socket file descriptor,
so the document "the time for socket read/write operations" is correct.

> ISTM that the implementation could make the cancelling as low as 1 second
> because of rounding. This could be said somewhere, maybe in the doc,
> surely in a comment.
It is said in doc. Right?
+The minimum allowed timeout is 2 seconds, so a value of 1 is
+interpreted as 2.
Or "because of rounding" is needed?

> I still think that this parameter should be preservered on psql's
> reconnections when explicitely set to non zero.
Would you please tell me your vision ?
Do you want to inherit all parameters with one option?
Or one to one?

[1] 
https://www.postgresql.org/message-id/0A3221C70F24FB45833433255569204D1FBC7FD1%40G01JPEXMBYT05
Best regards,
-
Ryohei Nagaura




socket_timeout_v12.patch
Description: socket_timeout_v12.patch


TCP_backend_v19.patch
Description: TCP_backend_v19.patch


TCP_interface_v19.patch
Description: TCP_interface_v19.patch


RE: Timeout parameters

2019-03-29 Thread Nagaura, Ryohei
Hi all.

I found my mistake in backend patch.

I modified from
+   port->keepalives_count = count;
to
+   port->tcp_user_timeout = timeout;
in line 113.

Sorry for mistake like this again and again...
Best regards,
-
Ryohei Nagaura




socket_timeout_v12.patch
Description: socket_timeout_v12.patch


TCP_backend_v18.patch
Description: TCP_backend_v18.patch


TCP_interface_v17.patch
Description: TCP_interface_v17.patch


RE: Timeout parameters

2019-03-29 Thread Nagaura, Ryohei
Hi Horiguchi-san,
Thank you so much for your review!

> From: Kyotaro HORIGUCHI [mailto:horiguchi.kyot...@lab.ntt.co.jp]
> Hmm. "forcefully" means powerful or assertive, in Japanese "力強く
> " or "強硬に".  "forcibly" means acoomplished through force, in Japanesee "
> 無理やり" or "強引に". Actually the latter is used in man 7 tcp.
Oops, I adopted the latter in both patches about tcp user timeout.

> The comment protrudes left.
Fixed.

> Couldn't we have the new variable in the section, like this?
ok!

Best regards,
-
Ryohei Nagaura




socket_timeout_v12.patch
Description: socket_timeout_v12.patch


TCP_backend_v19.patch
Description: TCP_backend_v19.patch


TCP_interface_v18.patch
Description: TCP_interface_v18.patch


RE: Timeout parameters

2019-03-29 Thread Nagaura, Ryohei
Hi all.

I found that connect_timeout uses pqWaitTimed().
Socket_timeout is related to pqWait() not pqWaitTimed().
Thus, I removed connect_timeout in my socket_Timeout patch.

FYI, I summarized a use case of this parameter.
The connection is built successfully.
Suppose that the server is hit by some kind of accident(e,g,. I or 
Tsunakawa-san suggested).
At this time, there is no guarantee that the server OS can pass control to the 
postmaster.
Therefore, it is considered natural way to disconnect the connection from the 
client side.
The place of implement of disconnection is pqWait() because it is where users' 
infinite wait occurs.

Best regards,
-
Ryohei Nagaura




socket_timeout_v12.patch
Description: socket_timeout_v12.patch


TCP_backend_v17.patch
Description: TCP_backend_v17.patch


TCP_interface_v17.patch
Description: TCP_interface_v17.patch


RE: Timeout parameters

2019-03-29 Thread Nagaura, Ryohei
Hi, Kirk-san,

> From: Jamison, Kirk [mailto:k.jami...@jp.fujitsu.com]
> In addition, regarding socket_timeout parameter.
> I referred to the doc in libpq.sgml, corrected misspellings, and rephrased the
> doc a little bit as below:
You aimed consistency with connect_timeout. Didn't you?
If yes, That is a nice idea!

Best regards,
-
Ryohei Nagaura



socket_timeout_v11.patch
Description: socket_timeout_v11.patch


TCP_backend_v17.patch
Description: TCP_backend_v17.patch


TCP_interface_v17.patch
Description: TCP_interface_v17.patch


RE: Timeout parameters

2019-03-28 Thread Nagaura, Ryohei
Hi, Tsunakawa-san, Kirk-san.

Thank you for your review.


Tsunakawa-san,
> From: Tsunakawa, Takayuki [mailto:tsunakawa.ta...@jp.fujitsu.com]
> The client-side tcp_user_timeout patch looks good.
Thanks, but I minorly changed that patch.
It is the declaration place of sebuf[], moving to just before where it'll 
become needed.


> The server-side tcp_user_timeout patch needs fixing the following:
> (1)
> + GUC_UNIT_MS | GUC_NOT_IN_SAMPLE
> + 12000, 0, INT_MAX,
> 
> GUC_NOT_IN_SAMPLE should be removed because the parameter appears in 
> postgresql.conf.sample.
> The default value should be 0, not 12000.
Oops, fixed!

> (2)
> Now that you've changed show_tcp_usertimeout() to use 
> pq_gettcpusertimeout(), you need to modify pq_settcpusertimeout(); just 
> immitate pq_setkeepalivesidle().
> Otherwise, SHOW would display a wrong value.
Indeed, pq_settcpusertimeout() should be modified as well.


> The socket_timeout patch needs the following fixes.  Now that others 
> have already tested these patches successfully, they appear committable to me.
Thank you so much for reviewing even socket_timeout patch.

> The reason why oom_error label is present is that it is used at 
> multiple places to avoid repeating the same error processing code.
Understood.

> (2)
> + conn->sock = -1;
> 
> Use PGINVALID_SOCKET instead of -1.
Oh, I see.


Kirk-san,
> I referred to the doc in libpq.sgml, corrected misspellings, and rephrased the
> doc a little bit as below:
Changed "stop-gap" and "mean" -> "stopgap" and "measure" respectively.
But I can't find the difference between mine and yours.
Would you please tell me if there is a difference like this depending on your 
rephrasing?

Best regards,
-
Ryohei Nagaura




socket_timeout_v10.patch
Description: socket_timeout_v10.patch


TCP_backend_v17.patch
Description: TCP_backend_v17.patch


TCP_interface_v17.patch
Description: TCP_interface_v17.patch


RE: Timeout parameters

2019-03-28 Thread Nagaura, Ryohei
Hello,

In the last client-side tcp user timeout patch:
+   appendPQExpBuffer(>errorMessage,
+   
libpq_gettext("setsockopt(TCP_USER_TIMEOUT) not supported: %s\n"),
+   SOCK_STRERROR(SOCK_ERRNO, 
sebuf, sizeof(sebuf)));
Keepalives* parameters in client-side libpq don't output error message like 
above even if the system don't support.
So, I removed this sentence in the current patch (TCP_interface).

Best regards,
-
Ryohei Nagaura




socket_timeout_v9.patch
Description: socket_timeout_v9.patch


TCP_backend_v16.patch
Description: TCP_backend_v16.patch


TCP_interface_v16.patch
Description: TCP_interface_v16.patch


RE: Timeout parameters

2019-03-28 Thread Nagaura, Ryohei
Hello, Kirk-san.

> From: Jamison, Kirk [mailto:k.jami...@jp.fujitsu.com]
> >In TCP_USER_TIMEOUT backend patch:
> > 1) linux ver 2.6.26 -> 2.6.36
> "Linux" should be capitalized.
Oh, yes. I see.

> In config.sgml it uses both "zero" and "0", while in libpq.sgml it only
> uses "zero".
> Since you patterned it from keepalives* docs for both files, I think it's
> alright doing it that way.
Thanks.

In addition, I modified tcp client side patch.
The last patch can't be compiled...
# declaration of sebuf in setTCPUserTimeout()
I'm very sorry for ridiculous mistake.

Best regards,
-
Ryohei Nagaura




socket_timeout_v9.patch
Description: socket_timeout_v9.patch


TCP_backend_v16.patch
Description: TCP_backend_v16.patch


TCP_interface_v15.patch
Description: TCP_interface_v15.patch


RE: Timeout parameters

2019-03-28 Thread Nagaura, Ryohei
Hello,

I updated my patches.

In TCP_USER_TIMEOUT backend patch:
1) linux ver 2.6.26 -> 2.6.36

2) error for systems where doesn't support this parameter

In TCP_USER_TIMEOUT interface patch:
error for systems where doesn't support this parameter

Best regards,
-
Ryohei Nagaura



TCP_interface_v14.patch
Description: TCP_interface_v14.patch


socket_timeout_v9.patch
Description: socket_timeout_v9.patch


TCP_backend_v15.patch
Description: TCP_backend_v15.patch


RE: Timeout parameters

2019-03-28 Thread Nagaura, Ryohei
Hi, all.

# This is a supplement of my current patches.

> From: Nagaura, Ryohei [mailto:nagaura.ryo...@jp.fujitsu.com]
> > In TCP_backend patch:
> > I think this is not mentioning backend. Why don't you copy'n paste
> > then modify the description of tcp_keepalives_idle? Perhaps it needs
> a
> > similar caveat related to Windows.
I modified documentation with referring ones of tcp_keepalives_*.

Also, Note that:
1) I added  about windows OS and some linux kernels.
2) Documentations about TCP_USER_TIMEOUT is made citing description of [1].
# The client-side patch does as well.
3) Same as keepalives*, I used both "0" and zero.
I can't recognize why to use both of them.
Is there anyone who know it?

[1] https://tools.ietf.org/html/rfc5482
Best regards,
-
Ryohei Nagaura







RE: Timeout parameters

2019-03-28 Thread Nagaura, Ryohei
Hi, Tsunakawa-san.

> From: Tsunakawa, Takayuki [mailto:tsunakawa.ta...@jp.fujitsu.com]
> I think that's for the case where the modules is built on an OS that
> supports TCP_USER_TIMEOUT (#ifdef TCP_USER_TIMEOUT is true), and the
> module is used on an older OS that doesn't support TCP_USER_TIMEOUT.  I
> remember I was sometimes able to do such a thing on Linux and Solaris.
> If we don't have to handle such usage, I agree about removing the special
> handling of ENOTPROTO.
I don't know how much there are such systems, but your story feels remote for 
me.
Therefore I adopted Horiguchi-san's idea.

Best regards,
-
Ryohei Nagaura







RE: Timeout parameters

2019-03-28 Thread Nagaura, Ryohei
Hello, Horiguchi-san.

Thank you for review.

> In TCP_backend patch:
> I think this is not mentioning backend. Why don't you copy'n paste then
> modify the description of tcp_keepalives_idle? Perhaps it needs a similar
> caveat related to Windows.

> +static const char*
> +show_tcp_user_timeout(void)
> The reason for, say, tcp_keepalive_idle uses the hook is that it doesn't
> show the GUC variable, but the actual value read by getsockopt. This is
> just showing the GUC value. I think this should behave the same way with
> tcp_keepalive* options. If not, we should have an explanation of the
> reason there.
Oh, Thank you for teaching.
I add a function pq_gettcpusertimeout() same as keepalives*.

> In TCP_interface patch:
> I would suggest the same thing as the backend part. Copy'n-paste-modify
> keepalives_idle would be better.
Same as backend-side, I made keepalives documentation reference.
I refered keepalives* documentation and modified my doc.

> I suppose that the reason ENOPROTOOPT is excluded from error condition
> is that the system call may fail with that errno on older kernels, but
> I don't think that that justifies ignoring the failure.
Thank you for your point!
Removed in both patches.

> I don't see a point in the added part having own "if (!IS_AF_UNIX" block
> separately from tcp_keepalive options.
Sorry, my coding was bad...
I integrated it with coding about keepalives.

> + /* TCP USER TIMEOUT */
> + {"tcp_user_timeout", NULL, NULL, NULL,
> + "TCP_user_timeout", "", 10, /* strlen(INT32_MAX) ==
> 10 */
> + offsetof(struct pg_conn, pgtcp_user_timeout)},
> +
> 
> Similary, why isn't this placed just after tcp_keepalives options?
Moved!

> +char   *tcp_user_timeout;/* tcp user timeout */
> The latter doesn't seem to be used.
This is also my bad coding...
Removed!

Best regards,
-
Ryohei Nagaura



TCP_backend_v14.patch
Description: TCP_backend_v14.patch


TCP_interface_v13.patch
Description: TCP_interface_v13.patch


socket_timeout_v9.patch
Description: socket_timeout_v9.patch


RE: Timeout parameters

2019-03-27 Thread Nagaura, Ryohei
Hello, Fabien-san.

> From: Fabien COELHO [mailto:coe...@cri.ensmp.fr]
> The default postgres configuration file should be updated to reflect the
> parameter and its default value.
I'm very sorry for not addressing your review in the last patch.
I modified TCP_backend patch (adding postgresql.conf.sample) and attached in 
this mail.

Best regards,
-
Ryohei Nagaura



socket_timeout_v9.patch
Description: socket_timeout_v9.patch


TCP_interface_v12.patch
Description: TCP_interface_v12.patch


TCP_backend_v13.patch
Description: TCP_backend_v13.patch


RE: Timeout parameters

2019-03-27 Thread Nagaura, Ryohei
Hello, Fabien-san.

> From: Fabien COELHO 
> About the backend v11 patch.
> No space or newline before ";". Same comment about the libpq_ timeout.
Fixed.

> There is an error in the code, I think it should be < 0 to detect errors.
Yes, thanks!

> If the parameter has no effect on Windows, I do not see why its value should 
> be
> constrained to zero, it should just have no effect. Idem libpq_ timeout.
I had a misunderstanding.
Indeed, it doesn't need to be zero. Removed.

> Documentation:
> ISTM this is not about libpq connections but about TCP connections. There can 
> be
> non libpq implementations client side.
Then, where do you think the correct place?
I thought that this parameter should be explained here because the communication
will be made through the library libpq same as keepalive features.


Best regards,
-
Ryohei Nagaura




socket_timeout_v9.patch
Description: socket_timeout_v9.patch


TCP_backend_v12.patch
Description: TCP_backend_v12.patch


TCP_interface_v12.patch
Description: TCP_interface_v12.patch


RE: Timeout parameters

2019-03-25 Thread Nagaura, Ryohei
Hi, kirk-san.

> From: Jamison, Kirk
> Ok. So I'd only take a look at TCP_USER_TIMEOUT parameter for now (this
> CommitFest), and maybe we could resume the discussion on socket_timeout
> in the future.
Yes, please.

> Your patch applies, however in TCP_backend_v10 patch, your documentation
> is missing a closing tag  so it could not be tested.
> When that's fixed, it passes the make check.
Oops! Fixed.

> what's the reason to emphasize Windows not supported in a separate
> paragraph?
I thought that it needs to be warned because there are some windows users.
But I came upon the idea that there are no need even it.
Thus the former doc you suggested seems better and I minor fixed doc to yours.


> (Note: I haven't checked which Linux versions are supported, I got it
> from your previous patch version.)
FYI, [1]

[1] http://man7.org/linux/man-pages/man7/tcp.7.html
Best regards,
-
Ryohei Nagaura



socket_timeout_v9.patch
Description: socket_timeout_v9.patch


TCP_backend_v11.patch
Description: TCP_backend_v11.patch


TCP_interface_v11.patch
Description: TCP_interface_v11.patch


RE: Timeout parameters

2019-03-24 Thread Nagaura, Ryohei
Hi,

First, thank you for your insightful discussion.
I remade patches and attached in this mail.

> From: Tsunakawa, Takayuki
> OTOH, it may be better to commit the tcp_user_timeout patch when
> Nagaura-san has refined the documentation, and then continue
> socket_timeout.
Yes, I want to commit TCP_USER_TIMEOUT patches in PG12.
Also I'd like to continue to discuss about socket_timeout after this CF.
# Thank you for your answering.


About TCP_USER_TIMEOUT:
1) Documentation and checking on UNIX system
As far as I surveyed (solaris and BSD family), these UNIX OS don't have 
"TCP_USER_TIMEOUT" parameter.
Accordingly I have not checked on real machine.
Also, I modified documentations to remove "equivalent to socket option"


As for socket_timeout:
1) documentation
> From: Robert Haas 
> This can't be used to force a global query timeout, because any kind of 
> protocol
> message - e.g. a NOTICE or WARNING - would reset the timeout, allowing the
> continue past the supposedly-global query timeout.  There may be some other
> ways that can happen, too, either currently or in the future.
Understood. Indeed, doc was not correct.

Mikalai-san,
You used to advise me about via UNIX domain sockets, but it turned out to be 
able to work.
Therefore, the documentation doesn't have the sentence about it.

Mikalai-san and Tsunakawa-san,
I modified documentation based on your comments about socket_timeout > other 
parameters.

2) checking whether int type
I Implemented in the current patch. Please see line 73 and 85 in the patch.

3) setting inheritance by "\c" command.
Inheritance of optional parameters such as this parameter,
(but not basic connection parameters such as dbname,)
should be implemented as one feature of "\c".
It is because, seen from users,
it is strange that some parameters can be taken over while the others can't.


Sorry for my late reply again and again...

Best regards,
-
Ryohei Nagaura



socket_timeout_v9.patch
Description: socket_timeout_v9.patch


TCP_backend_v10.patch.patch
Description: TCP_backend_v10.patch.patch


TCP_interface_v10.patch
Description: TCP_interface_v10.patch


RE: Timeout parameters

2019-03-13 Thread Nagaura, Ryohei
Hello Robert-san.

> From: Robert Haas 
> So this says that it works on systems that have TCP_USER_TIMEOUT or an
> equivalent socket option and that it also works on Windows, and then a few 
> lines
> later
> 
> +   This parameter is not supported on Windows, and must be zero.
> 
> This says it actually doesn't work on Windows.
It has been pointed out by Fabien-san before and it has not been corrected yet.
I am very sorry for making "needs review" even though I have not reposted the 
patch.

> I think the language about an equivalent socket option isn't helpful.
> We should document any equivalents we actually support, and not say anything
> about anything else.
Does it mean that doc explain with words "setsockopt()" and/or its option 
"TCP_USER_TIMEOUT" ?
BTW, This sentence is to be consistent with the keepalive description e.g., [1].
In my opinion, if this doc need to be changed, we should change docs about 
keepalive too.
How do you think?

> +   To enable full control under TCP connection use this option
> together with
> +   keepalive.
> That doesn't tell me anything useful.
Oh, I see. Indeed, this is not about postgres but general network.

> 
> 
> +Specify in milliseconds the time to disconnect to the client
> +when there is no ack packet from the client to the server's
> data transmission.
> +This parameter is supported on linux version 2.6.37 or later.
> 
> Hmm.  This looks like a second, and broadly better, definition of the 
> parameter.
> Now we have a different definition of where it's supported.  This is the THIRD
> attempt to tell me which platforms are supported -- just Linux, 2.6.37 or 
> greater.
> 
> + This parameter is not supported on Windows.
> 
> And then, in case I missed the last three attempts to tell me about platform
> support, there's this.
I intended to delete these sentences. I'll delete it.

> Have you checked whether this can be easily supported on FreeBSD and/or
> NetBSD?
No, not yet. Checked red hat enterprise linux only...
Indeed, I should have checked on some UNIX OS.

[1] https://www.postgresql.org/docs/current/runtime-config-connection.html
Best regards,
-
Ryohei Nagaura



RE: Timeout parameters

2019-03-13 Thread Nagaura, Ryohei
Hello Mikalai-san.

> From: mikalaike...@ibagroup.eu 
> The main idea of my comment was to avoid handling logical errors ( 
> "client-side
> timeout") in advance to the detection of network problems
> Therefore, I suggested setting "client-side timeout" greater of equal to the
> TCP_USER_TIMEOUT or note about it in the documentation.
I have caught up with your intension. You have a point! Thank you.

> I'm afraid to be mistaken but I think that when network is alive it means 
> that OS
> does not hang.
> I think that it is a responsibility of the OS kernel to manage network 
> connections.
> Keep_alive mechanism checks a network state by exchanging data between
> client's and server's OS kernels. I think the same procedure is used in
> TCP_USER_TIMEOUT mechanism. So, when keep_alive and
> TCP_USER_TIMEOUT say that network works correct, it means that kernels of
> client and server do not hang. On the other hand it does not guarantee that
> PostgreSQL server is not hang. "client-side timeout" should handle this 
> problem
> when network problem was not detected.
Thank you for your kind explain.

Best regards,
-
Ryohei Nagaura





RE: Timeout parameters

2019-03-11 Thread Nagaura, Ryohei
Hello, Mikalai-san.

Thank you for your mail.

> From: mikalaike...@ibagroup.eu 
> I'm with Fabien that "client-side timeout" seems unsafe. Also I agree with 
> Fabien
> that quire can take much time to be processed by the PosgtreSQL server and it 
> is
> a normal behavior. There is possible that performance of the PostgreSQL server
> machine can be low temporary or continuously, especially during upgrading
> procedure.
I'm very poor at English, so I couldn't read your intension.
In conclusion, you think this feature should process something e.g., sending 
canceling request. Don't you?
If yes, is it hard for you to accept my thought as follows?
1. The "end-user" I mentioned is application/system user.
2. Such end-users don't want to wait responses from system or application for 
whatever reason.
3. End-users don't care where the failure of the system is when it occurs.
4. They just don't want to wait long.
5. If I made the server processing something when this parameter is triggered, 
they would wait for the time it takes to process it.
6. Therefore it is not preferable to make servers processing something in this 
feature.

> Such a situation looks to be covered by TCP_USER_TIMEOUT and keep_alive
> mechanisms. 
i.e., you mean that it can't be happened to occur OS hang with network still 
alive.
Don't you?

These comments are helpful for documenting. Thank you.
> I think it is important to add some more information into the description of 
> this
> parameter which informs end-users that this parameter has to be used very
> carefully because it can impact as on the client application as on the server.
>...
> May be it is better to warn in documentation or prohibit in the source
> code to set "client-side timeout" less than TCP_USER_TIMEOUT to avoid handling
> "possible" logical problems ahead to the network problems.
> Keep in mind that "client-side timeout" can abort a connection which uses 
> UNIX-domain sockets too.
I didn't take into account it. I'll investigate it. Thanks.

Best regards,
-
Ryohei Nagaura





RE: Timeout parameters

2019-03-10 Thread Nagaura, Ryohei
Hi, Fabien-san.

> From: Fabien COELHO 
> As I understand the "client-side timeout" use-case, as distinct from
> network-issues-related timeouts proposed in the other patches, the point is 
> more
> or less to deal with an overloaded server.
The main purpose of this parameter is to avoid client's waiting for DB server 
infinitely, not reducing the server's burden.
This results in not waiting end-user, which is most important.
As Tsunakawa-san presented, there are similar parameters in JDBC and Oracle, so 
I think that this idea is valid.

> The timeout is raised because getting the result takes time, which may be 
> simply
> because the query really takes time, and it does not imply that the server 
> would
> not be able to process a cancel request.
You mentioned about when a SQL query is heavy, but I want to talk about when OS 
hang.
If OS hang occurs, the cost of cancel request processing is so high.
This is the reason why the implementation is just only disconnection without 
canceling.

Best regards,
-
Ryohei Nagaura





RE: Timeout parameters

2019-03-08 Thread Nagaura, Ryohei
Hi, Fabien-san.

About TCP_USER_TIMEOUT:
> From: Fabien COELHO 
> I could not really test the feature, i.e. I could not trigger a timeout.
> Do you have a suggestion on how to test it?
Please see the previous mail[1] or current kirk-san's mail.

About socket_timeout:
> From: Fabien COELHO 
> are actually finished. I cannot say that I'm thrilled by that behavior. ISTM 
> that libpq
> should at least attempt to cancel the query 
Would you please tell me why you think so? 
If it was implemented so, timeout couldn't work when the server is so busy that 
can't process a cancel request.
If the client encounters such a case, how does the client stop to wait the 
server?
How does the client release its resources?
Socket_timeout is helpful for such clients.

I'll send patches after next week.

[1] 
https://www.postgresql.org/message-id/EDA4195584F5064680D8130B1CA91C453A32DB@G01JPEXMBYT04
Best regards,
-
Ryohei Nagaura





RE: Timeout parameters

2019-02-27 Thread Nagaura, Ryohei
Hi,

I rewrote two TCP_USER_TIMEOUT patches.
I changed the third argument of setsockopt() from 18 to TCP_USER_TIMEOUT.

This revision has the following two merits.
* Improve readability of source
* Even if the definition of TCP_USER_TIMEOUT is changed, it is not affected.
  e.g., in the current linux, TCP_USER_TIMEOUT is defined, but even if it is 
changed to 19, it 'll be available.

Best regards,
-
Ryohei Nagaura



TCP_backend_v8.patch
Description: TCP_backend_v8.patch


TCP_interface_v8.patch
Description: TCP_interface_v8.patch


socket_timeout_v7.patch
Description: socket_timeout_v7.patch


RE: Timeout parameters

2019-02-26 Thread Nagaura, Ryohei
Hi, kirk-san.

Thank you for review.

> From: Jamison, Kirk 
> Your socket_timeout patch still does not apply either with git or patch 
> command. It
> says it's still corrupted.
> I'm not sure about the workaround, because the --ignore-space-change and
> --ignore-whitespace did not work for me.
> Maybe it might have something to do with your editor when creating the patch.
> Could you confirm?
> The CFbot is also another way to check if your latest patches work.
> http://commitfest.cputube.org/
It may be caused by git diff command with no option.
The new patches are created by git diff -w.

About socket_timeout:
Your feedback looks good so that I adopted it. Please confirm it.

About TCP_USER_TIMEOUT:
There is no change in sources.
Just change the command option "git diff" -> "git diff -w".

Best regards,
-
Ryohei Nagaura



TCP_backend_v7.patch
Description: TCP_backend_v7.patch


TCP_interface_v7.patch
Description: TCP_interface_v7.patch


socket_timeout_v7.patch
Description: socket_timeout_v7.patch


RE: Timeout parameters

2019-02-26 Thread Nagaura, Ryohei
This mail is the same as 
https://www.postgresql.org/message-id/EDA4195584F5064680D8130B1CA91C453EBE79%40G01JPEXMBYT04
I resend because the mail didn't be reflected.

Hi, kirk-san.

> From: Nagaura, Ryohei  This is my bad. 
> I'll remake it.
> Very sorry for the same mistake.
I remade the patches and attached in this mail.
In socket_timeout patch, I replaced "atoi" to "parse_int_param" and inserted 
spaces just after some comma.
There are a few changes about documentation for the following reason:

> From: Jamison, Kirk  Got the doc fix. I 
> wonder if we need to document what effect the parameter does:
> terminating the connection. How about:
I also don't know, but...

> Controls the number of seconds of client-server communication 
> inactivity before forcibly closing the connection in order to prevent 
> client from infinite waiting for individual socket read/write 
> operations. This can be used both as a force global query timeout and 
> network problems detector, i.e. hardware failure and dead connection. A value 
> of zero (the default) turns this off.
"communication inactivity" seems to be a little extreme.
If the communication layer is truly dead you will use keepalive.
This use case is when socket option is not available for some reason.
So it would be better "terminating the connection" in my thought.
And...

> Well, you may remove the "i.e. hardware failure and dead connection" 
> if that's not necessary.
I don't think it is necessary because you can use this parameter other than 
that situations.
Not "i.e." but "e.g." have a few chance to be documented.

About TCP_USER_TIMEOUT patches, there are only miscellaneous changes: removing 
trailing spaces and making comments of parameters lower case as you pointed out.

Best regards,
-
Ryohei Nagaura



TCP_interface_v6.patch
Description: TCP_interface_v6.patch


TCP_backend_v6.patch
Description: TCP_backend_v6.patch


socket_timeout_v6.patch
Description: socket_timeout_v6.patch


RE: Timeout parameters

2019-02-25 Thread Nagaura, Ryohei
Hi, kirk-san.

> From: Jamison, Kirk 
> $ git apply ../socket_timeout_v5.patch
> fatal: corrupt patch at line 24
> --> l24: diff --git a/src/interfaces/libpq/fe-connect.c
> --> b/src/interfaces/libpq/fe-connect.c
How about applying by "patch -p1"?
I have confirmed that my patch could be applied in that way while not confirmed 
using "git apply" command.

> a. Use parse_int_param instead of atoi
> >+conn->socket_timeout = atoi(conn->pgsocket_timeout);
This is my bad. I'll remake it.
Very sorry for the same mistake.


Best regards,
-
Ryohei Nagaura 




RE: Timeout parameters

2019-02-24 Thread Nagaura, Ryohei
Hi, all.

Thank you for discussion.
I made documentation about socket_timeout and reflected Tsunakawa-san's comment 
in the new patch.
# Mainly only fixing documentation...
The current documentation of socket_timeout is as follows:
socket_timeout
Controls the number of second of client's waiting time for individual socket 
read/write operations. This can be used both as a force 
global query timeout and network problems detector. A value of zero (the 
default) turns this off.

Best regards,
-
Ryohei Nagaura



TCP_backend_v5.patch
Description: TCP_backend_v5.patch


TCP_interface_v5.patch
Description: TCP_interface_v5.patch


socket_timeout_v5.patch
Description: socket_timeout_v5.patch


RE: Timeout parameters

2019-02-20 Thread Nagaura, Ryohei
Hi, Tsunakawa-san

Thank you for your comments.

On Wed, Feb 20, 2019 at 5:56 PM, Tsunakawa, Takayuki wrote:
> > Perhaps you could also clarify a bit more through documentation on how
> > socket_timeout handles the timeout differently from statement_timeout
> > and tcp_user_timeout.
> Maybe.  Could you suggest good description?
Clients wait until the socket become readable when they try to get results of 
their query.
If the socket state get readable, clients read results.
(See src/interfaces/libpq/fe-exec.c, fe-misc.c)
The current pg uses poll() to monitor socket states.
"socket_timeout" is used as timeout argument of poll().
(See [1] about poll() and its arguments)

Because "tcp_user_timeout" handles the timeout before and after the above 
procedure,
there are different monitoring target between "socket_timeout" and 
"tcp_user_timeout".
When the server is saturated, "statement_timeout" doesn't work while 
"socket_timeout" does work.
In addition to this, the purpose of "statement_timeout" is to reduce server's 
burden, I think.
My proposal is to enable clients to release their resource which is used 
communication w/ the saturated server.

On Wed, Feb 20, 2019 at 7:10 PM, Tsunakawa, Takayuki wrote:
> a) is not always accurate, because libpq is also used in the server.  For 
> example,
> postgres_fdw and WAL receiver in streaming replication.
I didn't take that possibility into consideration.
Certainly, a) is bad.

> I'm OK with either the current naming or b).  Frankly, I felt a bit strange 
> when I
> first saw the keepalive parameters, wondering why the same names were not
> chosen.
I will refer to it and wait for opinion of the reviewer.

[1] http://man7.org/linux/man-pages/man2/poll.2.html
Best regards,
-
Ryohei Nagaura




RE: Timeout parameters

2019-02-20 Thread Nagaura, Ryohei
Hi, Kirk-san.

Thank you for summarizing this thread.

On Tue, Feb 19, 2019 at 6:05 PM, Jamison, Kirk wrote:
> 1) tcp_user_timeout parameter
> As for user_timeout param, there seems to be a common agreement with regards
> to its need.
> I think this can be "committed" separately when it's finalized.
I also recognize so.
BTW, tcp_user_timeout parameter of servers and clients have same name in my 
current implementation.
I think it would be better different name rather than same name.
I'll name them as the following a) or b):
a) server_tcp_user_timeout and client_tcp_user_timeout
b) tcp_user_timeout and user_timeout
b) is the same as the naming convention of keepalive, but it is not 
user-friendly. 
Do you come up with better name?
Or opinion?

> (2) tcp_socket_timeout (or suggested client_statement_timeout,
> send_timeout/recv_timeout)
> Perhaps you could also clarify a bit more through documentation on how
> socket_timeout handles the timeout differently from statement_timeout and
> tcp_user_timeout.
> Then we can decide on the which parameter name is better once the
> implementation becomes clearer.
* The following use case is somewhat different from those listed first. Sorry.
Use case:
1) A client query to the server and the statement is delivered correctly.
2) The server become saturated.
3) But the network layor is alive.
Because of 1), tcp_user_timeout doesn't work.
Because of 2), statement_timeout doesn't work.
Because of 3), keepalive doesn't work.
In the result, clients can't release their resource despite that they want.

My suggestion is this solution.

To limit user waiting time in pqWait(), I implemented some same logic of 
pqWaitTimed().

Best regards,
-
Ryohei Nagaura




RE: Timeout parameters

2019-02-06 Thread Nagaura, Ryohei
Hi Fabien,

Would you review TCP_USER_TIMEOUT patches first please?
I want to avoid the situation that 
the discussion of socket_timeout has been lengthened 
and tcp_user_timeout patch is also not commit in the next CF.

On Mon, Feb 4, 2019 at 2:24 AM, Michael Paquier wrote:
> Moved to next CF per the latest updates: there is a patch with no reviews for 
> it.
Thank you.

Best regards,
-
Ryohei Nagaura





RE: [HACKERS] Cached plans and statement generalization

2019-01-29 Thread Nagaura, Ryohei
Hi,

On Mon, Jan 28, 2019 at 10:46 PM, Konstantin Knizhnik wrote:
> Rebased version of the patch is attached.
Thank you for your quick rebase.

> This files are just output of test execution and it is not possible to expect 
> lack of
> trailing spaces in output of test scripts execution.
I understand this is because regression tests use exact matching.
I learned a lot. Thanks.

Best regards,
-
Ryohei Nagaura



RE: [HACKERS] Cached plans and statement generalization

2019-01-28 Thread Nagaura, Ryohei
Hi,

Although I became your reviewer, it seems to be difficult to feedback in this 
CF.
I continue to review, so would you update your patch please?
Until then I review your current patch.

There is one question.
date_1.out which maybe is copy of date.out includes trailing space and gaps of 
indent
e.g., line 3368 and 3380 in your current patch have 
space in each end of line
different indent.
This is also seen in date.out.
I'm not sure whether it is ok to add new file including the above features just 
because a existing file includes.
Is it ok?

Best regards,
-
Ryohei Nagaura


RE: Timeout parameters

2019-01-27 Thread Nagaura, Ryohei
Hi,

Sorry for my late.

On Tue, Dec 25, 2018 at 7:40 PM, Fabien COELHO wrote:
> I still do not understand the use-case specifics: for me, aborting the
> connection, or a softer cancelling the statement, will result in the server
> stopping the statement, so the server does NOT "continue the job", 
The server continue the job when the connection is aborted.
You can confirm by the following procedure.
0. connect to the server normally.
1. query the following statement
 =# update tbl set clmn = (select to_number(pg_sleep(10)||'10','20'));
2. kill client-side psql process before the result returns.
3. reconnect the server and "select" query on tbl.

On Thu, Jan 10, 2019 at 3:14 AM, ayaho...@ibagroup.eu wrote:
> Takayuki Tsunakawa, could you provide wider explanation of socket_timeout?
> I'm little bit misunderstanding in which cases this parameter is/can be
> used.
The communication between a client and the server is normal.
The server is so busy that the server can return ack packet and can't work 
statement_timeout.
In this case, the client may wait for very long time.
This parameter is effective for such clients.

> If TCP_USER_TIMEOUT is not supported by PostgreSQL, it means that TCP
> connection are partly controlled by the operation system (kernel). In this
> case pqWaitTimed() should be used on the application layer for connection
> control in data transmission phase.
In the current postgres, PQgetResult() called by sync command "PQexec()" uses 
pqWait().
If the user wishes to sync communication, how do you specify the waiting time 
limit?
It makes sense to implement in pqWait() that can wait clients indefinitely, I 
think.

> As for me It better to specify the description as follows:
Thank you for your comment.
I adopted your documentation in the current patch.

On Wed, Dec 26, 2018 at 8:25 PM, Tsunakawa, Takayuki wrote:
> To wrap up, the relevant parameters work like this:
> 
> * TCP keepalive and TCP user (retransmission) timeout: for network problems
> * statement_timeout: for long-running queries
> * socket_timeout (or send/recv_timeout): for saturated servers
Thank you for your summary.


Best regards,
-
Ryohei Nagaura



TCP_backend_v4.patch
Description: TCP_backend_v4.patch


TCP_interface_v4.patch
Description: TCP_interface_v4.patch


RE: libpq debug log

2019-01-24 Thread Nagaura, Ryohei
Hi Iwata-san,

I used your patch for my private work, so I write my opinion and four feedback 
below.
On Fri, Jan 18, 2019 at 8:19 AM, Iwata, Aya wrote:
> - Setting whether to get log or not by using connection strings or environment
> variables. It means that application source code changes is not needed to get
> the log.
> - Getting time when receive and send process start/end. Functions too.
This merit was very helpful for my use, so I want your proposal function in 
postgres.

The followings are feedback from me.

1)
It would be better making the log format the same as the server log format, I 
think.
Your log format:
2019/01/22 04:15:25.496 ...
Server log format:
2019-01-22 04:15:25.496 UTC ...
There are two differences:
One is separator character of date, "/" and "-".
The another is standard time information.

2)
It was difficult for me to understand the first line message in the log file.
"Max log size is 10B, log min level is LEVEL1"
Does this mean as follows?
"The maximum size of this file is 10 Bytes, the parameter 'log min level' is 
set to LEVEL 1."

3)
Under the circumstance that the environment variables "PGLOGDIR" and 
"PGLOGSIZE" are set correctly,
the log file will also be created when the user connect the server with "psql".
Does this follow the specification you have thought?
Is there any option to unset only in that session when you want to connect with 
"psql"?

4)
Your patch affects the behavior of PQtrace().
The log of the existing PQtrace() is as follows:
From backend> "id"
From backend (#4)> 16387
From backend (#2)> 1
From backend (#4)> 23
...
Your patch makes PQtrace() including the following log in addition to the above.
To backend> Msg complete, length 27
Start sending message to backend:End sending message to backend:PQsendQuery end 
:PQgetResult start :Start receiving message from backend:End receiving message 
from backend:From backend> T
...


For your information.
Best regards,
-
Ryohei Nagaura




[information] Winter vacation

2018-12-26 Thread Nagaura, Ryohei
Hi all.

-Information-
Members of Fujitsu Japan may not be able to reply in the term below.
TERM: 29th December 2018 ~ 6th January 2019

NOTE:
Members of Fujitsu Japan are those whose mail domain is "@jp.fujitsu.com"

Best regards,
-
Ryohei Nagaura





RE: Timeout parameters

2018-12-25 Thread Nagaura, Ryohei
Hi Fabien.

On Wed, Dec 26, 2018 at 3:02 AM, Fabien COELHO wrote:
> About the patches: you are expected to send consistent patches, i.e. one
> feature with its associated documentation, not two separate features and
> another patch for documenting them.
Thank you for teaching me.
I rewrote patches and attached in this mail.

Best regards,
-
Ryohei Nagaura



TCP_backend_v3.patch
Description: TCP_backend_v3.patch


TCP_interface_v3.patch
Description: TCP_interface_v3.patch


RE: Timeout parameters

2018-12-25 Thread Nagaura, Ryohei
Hi,

On Tue, Dec 25, 2018 at 2:59 AM, Fabien COELHO wrote:
> >> 4. The client wants to close the connection while leaving the job to
> >> the server.
> >> In this case, "statement_timeout" can't satisfy at line 4.
>
> Why?
> ISTM that "leaving the job" to the server with a client-side connection
> closed is basically an abort, no different from what server-side
> "statement_timeout" already provides?
"while leaving the job to the server" means that "while the server continue the 
job".
# Sorry for the inappropriate explanation.
I understand that "statement_timeout" won't.

> Also, from a client perspective, if you use statement_timeout, it would
> timeout, then the client would process the error and the connection would
> be ready for the next query without needing to be re-created, which is quite
> costly anyway? Also, if the server is busy, recreating an connection is
> expensive so it won't help much, really?
When the recreating the connection the server may be not busy.
In this case, it isn't so costly to reconnect.
Also, if a client do not have to execute the remaining query immediately after 
timeout, 
the client will have the choice of waiting until the server is not busy.

> About the implementation, I'm wondering whether something simpler could
> be done. Check how psql implements "ctrl-c" to abort a running query: it
> seems that it sends a cancel message, no need to actually abort the
> connection?
This is my homework.

> Hmmm "client_statement_timeout" maybe?
I agree.

Best regards,
-
Ryohei Nagaura





RE: [suggestion]support UNICODE host variables in ECPG

2018-12-25 Thread Nagaura, Ryohei
Hi,

On Fri, Dec 21, 2018 at 5:08 PM, Tom Lane wrote:
> I don't think I buy that argument; it falls down as soon as you consider
> characters above U+.  I worry that by supporting UTF16, we'd basically
> be encouraging users to write code that fails on such characters, which
> doesn't seem like good project policy.
Oh, I mistook.
Thank you for pointing out.

On Mon, Dec 24, 2018 at 5:07 PM, Matsumura Ryo wrote:
> I think that the first benefit of suggestion is providing a way to treat
> UTF16 chars for application. Whether or not to support above
> U+ (e.g. surrogate pair) may be a next discussion.
Thank you for your comments.
Yes, I'd like to judge the necessity of this function before designing.

Best regards,
-
Ryohei Nagaura






RE: [suggestion]support UNICODE host variables in ECPG

2018-12-21 Thread Nagaura, Ryohei
Matsumura-san, Tsunakawa-san

Thank you for reply.

Tsunakawa-san
> * What's the benefit of supporting UTF16 in host variables?
There are two benefits.
1) As byte per character is constant in UTF16 encoding, it can process strings 
more efficiently than other encodings.
2) This enables C programmers to use wide characters.

> * Does your proposal comply with the SQL standard?  If not, what does the
> SQL standard say about support for UTF16?
I referred to the document, but I could not find it.
Does anyone know about this?

> * Why only Windows?
It should be implemented in other OS if needed.

Matsumura-san
> I understand that the previous discussion pointed that the feature had
> better be implemented more simply or step-by-step and description about
> implementation is needed more.
> I also think it prevented the discussion to reach to the detail of feature.
> What is your opinion about it?
I wanted to discuss the necessity first, so I did not answer.
I'm very sorry for not having mentioned it.
If it is judged that this function is necessary, I'll remake the design.

Best regards,
-
Ryohei Nagaura




RE: Timeout parameters

2018-12-20 Thread Nagaura, Ryohei
Hi, Fabien.

The next CF will start so I want to restart the discussion.

> About "socket_timeout"
> If you face the following situation, this parameter will be needed.
If you feel that this situation can't happen or the use case is too limited, 
please point out so.

> > I think that there is some kind of a misnomer: this is not a
> > socket-level timeout, but a client-side query timeout, so it should be
> named differently?
> Yes, I think so.
> 
> > I'm not sure how to name it, though.
> Me too.
Since I want to use the monitoring target as the parameter name, let's decide 
the parameter name while designing.

> > I think that the way it works is a little extreme, basically the
> > connection is aborted from within pqWait, and then restarted from scratch.
Which motion seems to be uncomfortable?
Or both?

> > There is no clear way to know about the value of the setting (SHOW,
> > \set, \pset...).
That is a nice idea!
If this parameter implementation is decide, I'll also add these features.

> About "TCP_USER_TIMEOUT"
> I introduce the test methods of TCP_USER_TIMEOUT.
I only came up with this test methods with "iptables".
Since this command can be used only by root, I didn't create a script.

Best regards,
-
Ryohei Nagaura





[suggestion]support UNICODE host variables in ECPG

2018-12-17 Thread Nagaura, Ryohei
Hi all.

There is a demand for ECPG to support UNICODE host variables.
This topic has also appeared in thread [1].
I would like to discuss whether to support in postgres.

Do you have any opinion?

The specifications and usage described below are the same as in [1].

Requirements

1. support utext keyword in ECPG



The utext is used to define the Unicode host variable in ECPG application
in windows platform.



2. support  UVARCHAR keyword in ECPG



The UVARCHAR is used to define the Unicode vary length host variable in
ECPG application in windows platform.



3. Saving the content of the Unicode variables into the database as
database character set or getting the content from the database into the
Unicode variables.



4. Firstly can only consider the UTF8 as database character set and UTF16
as the Unicode format for host variable. A datatype convert will be done
between the UTF8 and UTF16 by ECPG.



5. Since Unicode has big-endian and little-endian format, a environment
variable is used to identify them and do the endianness convert accordingly.



Usage

int main() {
EXEC SQL BEGIN DECLARE SECTION;
utext employee[20] ;/* define Unicode host variable  */
UVARCHAR address[50] ;  /* defin a vary length Unicode host
variable  */
EXEC SQL END DECLARE SECTION;



...



EXEC SQL CREATE TABLE emp (ename char(20), address varchar(50));



/* UTF8 is the database character set  */
EXEC SQL INSERT INTO emp (ename) VALUES ('Mike', '1 sydney, NSW') ;



/* Database character set converted to Unicode */
EXEC SQL SELECT ename INTO :employee FROM emp ;



/* Database character set converted to Unicode */
EXEC SQL SELECT address INTO :address FROM emp ;



wprintf(L"employee name is %s\n",employee);



wprintf(L"employee address is %s\n", address.attr);



DELETE * FROM emp;



/* Unicode converted to Database character */
EXEC SQL INSERT INTO emp (ename,address) VALUES (:employee, :address);



EXEC SQL DROP TABLE emp;
EXEC SQL DISCONNECT ALL;
 }

[1]
https://www.postgresql.org/message-id/flat/CAF3%2BxMLcare1QrDzTxP-3JZyH5SXRkGzNUf-khSgPfmpQpkz%2BA%40mail.gmail.com

Best regards,
-
Ryohei Nagaura





RE: Timeout parameters

2018-12-06 Thread Nagaura, Ryohei
Hi,

There was an invisible space, so I removed it.
I registered with 2019-01 commitfest.

Best regards,
-
Ryohei Nagaura

> -Original Message-
> From: Nagaura, Ryohei [mailto:nagaura.ryo...@jp.fujitsu.com]
> Sent: Thursday, December 6, 2018 2:20 PM
> To: 'Fabien COELHO' ;
> 'pgsql-hack...@postgresql.org' 
> Cc: Yahorau, A. (IBA) 
> Subject: RE: Timeout parameters
> 
> Hi, Fabien.
> 
> Thank you for your review.
> And I'm very sorry to have kept you waiting so long.
> 
> 
> About "socket_timeout"
> 
> > I'm generally fine with giving more access to low-level parameters to
> users.
> > However, I'm not sure I understand the use case you have that needs
> > these new extensions.
> If you face the following situation, this parameter will be needed.
> 1. The connection between the server and the client has been established
> normally.
> 2. A server process has been received SQL statement.
> 3. The server OS can return an ack packet, but it takes time to execute
> the SQL statement
>Or return the result because the server process is very busy.
> 4. The client wants to close the connection while leaving the job to the
> server.
> In this case, "statement_timeout" can't satisfy at line 4.
> 
> > I think that there is some kind of a misnomer: this is not a
> > socket-level timeout, but a client-side query timeout, so it should be
> named differently?
> Yes, I think so.
> 
> > I'm not sure how to name it, though.
> Me too.
> 
> > I think that the way it works is a little extreme, basically the
> > connection is aborted from within pqWait, and then restarted from scratch.
> >
> > There is no clear way to know about the value of the setting (SHOW,
> > \set, \pset...). Ok, this is already the case of other connection
> parameters.
> If this parameter can be needed, I would like to discuss design and optional
> functions.
> How do you think?
> I'll correct patch of "socket_timeout" after that.
> 
> 
> About "TCP_USER_TIMEOUT"
> I fixed on the previous feedback.
> Would you review, please?
> 
> > There are no tests.
> I introduce the test methods of TCP_USER_TIMEOUT.
> 
> Test of client-side TCP_USER_TIMEOUT:
> [client operation]
> 1. Connect DB server.
>   postgres=# psql
> postgresql://USERNAME:PASSWORD@hostname:port/dbname?tcp_user_timeout=1
> 5000
> 2. Get the port number by the following command:
>   postgres=# select inet_client_port();
> 3. Close the client port from the other console of the client machine.
>Please rewrite "56750" to the number confirmed on line 2.
>   $ iptables -I INPUT -p tcp --dport 56750 -j DROP 4. Query the
> following SQL:
>   postgres=# select pg_sleep(10);
> 5. TCP USER TIMEOUT works correctly if an error message is output to the
> console.
> 
> Test of server-side TCP_USER_TIMEOUT:
> [client operation]
> 1. Connect DB server.
> 2. Get the port number by the following command:
>   postgres=# select inet_client_port();
> 3. Set the TCP_USER_TIMEOUT by the following command:
>   postgres=# set tcp_user_timeout=15000;
> 4. Query the following SQL:
>   postgres=# select pg_sleep(10);
> 5. Close the client port from the other console.
>Please rewrite "56750" to the number confirmed on line 2.
>   $ iptables -I INPUT -p tcp --dport 56750 -j DROP [server operation]
> 6. Verify the logfile.
> 
> > There is no documentation.
> I made a patch of documentation of TCP USER TIMEOUT.
> 
> Best regards,
> -
> Ryohei Nagaura



document_v2.patch
Description: document_v2.patch


TCP_backend_v2.patch
Description: TCP_backend_v2.patch


TCP_interface_v2.patch
Description: TCP_interface_v2.patch


RE: Timeout parameters

2018-12-05 Thread Nagaura, Ryohei
Hi, Fabien.

Thank you for your review.
And I'm very sorry to have kept you waiting so long.


About "socket_timeout"

> I'm generally fine with giving more access to low-level parameters to users.
> However, I'm not sure I understand the use case you have that needs these
> new extensions.
If you face the following situation, this parameter will be needed.
1. The connection between the server and the client has been established 
normally.
2. A server process has been received SQL statement.
3. The server OS can return an ack packet, but it takes time to execute the SQL 
statement 
   Or return the result because the server process is very busy.
4. The client wants to close the connection while leaving the job to the server.
In this case, "statement_timeout" can't satisfy at line 4.

> I think that there is some kind of a misnomer: this is not a socket-level
> timeout, but a client-side query timeout, so it should be named differently?
Yes, I think so.

> I'm not sure how to name it, though.
Me too.

> I think that the way it works is a little extreme, basically the connection
> is aborted from within pqWait, and then restarted from scratch.
>
> There is no clear way to know about the value of the setting (SHOW, \set,
> \pset...). Ok, this is already the case of other connection parameters.
If this parameter can be needed, I would like to discuss design and optional 
functions.
How do you think?
I'll correct patch of "socket_timeout" after that.


About "TCP_USER_TIMEOUT"
I fixed on the previous feedback.
Would you review, please?

> There are no tests.
I introduce the test methods of TCP_USER_TIMEOUT.

Test of client-side TCP_USER_TIMEOUT:
[client operation]
1. Connect DB server.
postgres=# psql 
postgresql://USERNAME:PASSWORD@hostname:port/dbname?tcp_user_timeout=15000
2. Get the port number by the following command:
postgres=# select inet_client_port();
3. Close the client port from the other console of the client machine.
   Please rewrite "56750" to the number confirmed on line 2.
$ iptables -I INPUT -p tcp --dport 56750 -j DROP
4. Query the following SQL:
postgres=# select pg_sleep(10);
5. TCP USER TIMEOUT works correctly if an error message is output to the 
console.

Test of server-side TCP_USER_TIMEOUT:
[client operation]
1. Connect DB server.
2. Get the port number by the following command:
postgres=# select inet_client_port();
3. Set the TCP_USER_TIMEOUT by the following command:
postgres=# set tcp_user_timeout=15000;
4. Query the following SQL:
postgres=# select pg_sleep(10);
5. Close the client port from the other console.
   Please rewrite "56750" to the number confirmed on line 2.
$ iptables -I INPUT -p tcp --dport 56750 -j DROP
[server operation]
6. Verify the logfile.

> There is no documentation.
I made a patch of documentation of TCP USER TIMEOUT.

Best regards,
-
Ryohei Nagaura



document.patch
Description: document.patch


TCP_backend.patch
Description: TCP_backend.patch


TCP_interface.patch
Description: TCP_interface.patch


RE: Timeout parameters

2018-10-31 Thread Nagaura, Ryohei
Hi Andrei,

First, I inform you that I may not contact for the following period:
From November 1st to November 19th

Second, I noticed my misunderstanding in previous mail.
> > Nevertheless, it is necessary to take into account that the option
> > TCP_USER_TIMEOUT is supported by Linux kernel starting since 2.6.37.
> > In a lower kernel version these changes will not take affect.
> Does it mean how do we support Linux OS whose kernel version is less than
> 2.6.37?
I understand that you pointed out my implementation.
I'll remake patch files when I return.

Finally, I write test method for each parameters here roughly.
You may use iptables command on linux when testing TCP_USER_TIMEOUT.
You may use pg_sleep(seconds) command in postgres.
I'll write the details after my returning.

Continue to discuss the socket_timeout, please.

Best regards,
-
Ryohei Nagaura



RE: Timeout parameters

2018-10-24 Thread Nagaura, Ryohei
Hi Andrei,

Thank you for response.

> TCP_USER_TIMEOUT option helps to overcome this problem and I agree with
> you that it needs to be supported within PostgreSQL.
I'm glad to your agreement.

> Nevertheless, it is necessary to take into account that the option
> TCP_USER_TIMEOUT is supported by Linux kernel starting since 2.6.37. In
> a lower kernel version these changes will not take affect.
Does it mean how do we support Linux OS whose kernel version is less than 
2.6.37?

> I am not sure that suggested by you “socket_timeout” option should be
> implemented.
> As a workaround I suggest using asynchronous command processing
> https://www.postgresql.org/docs/10/static/libpq-async.html
There are many applications implemented with synchronous API
(e.g. PQexec()), so "socket_timeout" is useful I think.

Best regards,
-
Ryohei Nagaura



Timeout parameters

2018-10-22 Thread Nagaura, Ryohei
Hi, all.

I'd like to suggest introducing two parameters to handle client-server 
communication timeouts.
That is "tcp_user_timeout" and "socket_timeout" parameter.

I implemented "tcp_user_timeout" parameter 
in both backend and frontend side.
This parameter enables us to 
use TCP_USER_TIMEOUT option on linux.
If the parameter is specified, the process sets the value to
TCP_USER_TIMEOUT option.
In my opinion, this option is needed for the following situation:
If the server can't return an ack packet to the request from the client, 
the client performs retransmission processing.
In this case TCP keepalive option can't work.
Therefore we need TCP USER TIMEOUT option.
Andrei Yahorau also refer to the necessity of this option in [1].

"socket_timeout" is the application layer timeout parameter 
from when frontend issues SQL query 
to when frontend receives the execution result from backend.
When this parameter is active and timeout occurs, 
frontend close the socket.
It is a merit for client to set the maximum time 
to wait for SQL.

I'm waiting for your opinions or reviews.

[1] 
https://www.postgresql.org/message-id/flat/OF4C8A68CE.A350F319-ON432582D0.0028A5FF-432582D0.002FEE28%40iba.by

Bes regards,
-
Ryohei Nagaura



socket_timeout.patch
Description: socket_timeout.patch


TCP_USER_TIMEOUT_in_backend.patch
Description: TCP_USER_TIMEOUT_in_backend.patch


TCP_USER_TIMEOUT_in_interface.patch
Description: TCP_USER_TIMEOUT_in_interface.patch