Expected behavior of context cancellation of in-progress query

[Jack Christensen]

I've been experimenting with the context features of 1.8 for github.com/jackc/pgx and have run across some surprising behavior.

When a context is canceled while a query is reading rows I observe the following behavior:

• rows.Err() is always <nil> even though query was interrupted. How do I detect a partial read of result set?
• Undefined number of additional rows read after context cancellation.
• Scan may or may not fail (this may be reasonable behavior).

I was able to reproduce this on Go 1.8rc3 with both pgx and pq.

Test case: https://github.com/jackc/context-rows-cancel

Is this expected behavior? Inability to detect an interrupted read would be undesirable. I'd also consider the extra rows read after cancellation to be undesirable, but not so serious.

Jack

[Daniel Theophanes]

Neither driver directly supports context yet. As such only the result is closed when it returns.

--
You received this message because you are subscribed to the Google Groups "golang-sql" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-sql+...@googlegroups.com.
To post to this group, send email to golan...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-sql/3c5629ec-f820-4648-49cb-be3389184e05%40jackchristensen.com.
For more options, visit https://groups.google.com/d/optout.

[Daniel Theophanes]

Just to flush out what I said before (I was on mobile), the package docs state:

> Drivers that do not support context cancelation will not return until after the query is completed.

In this case neither of the stated drivers support the Context methods. As such the sql package does not return until the query returns.

An alternate design would have been to return right after a cancel was received and discard the connection after it was returned behind the control flow, but that would leave the door open to connection pool exhaustion. I would love to see both of these drivers support the Context method and cancelation.

Thanks, -Daniel

[Jack Christensen]

Daniel Theophanes wrote:

Just to flush out what I said before (I was on mobile), the package docs state:

> Drivers that do not support context cancelation will not return until after the query is completed.

In this case neither of the stated drivers support the Context methods. As such the sql package does not return until the query returns.

An alternate design would have been to return right after a cancel was received and discard the connection after it was returned behind the control flow, but that would leave the door open to connection pool exhaustion. I would love to see both of these drivers support the Context method and cancelation.

I'm working on context support for pgx. This test was partially to flesh out what desired/correct behavior. I still think that inability to detect partial read is a pretty serious deficiency regardless of underlying driver behavior.

As far as correct driver behavior, is the correct response to a context cancellation to return ctx.Err() from whatever operation is interrupted?

Jack

Thanks, -Daniel

On Saturday, February 4, 2017 at 7:05:12 PM UTC-8, jack wrote:

I've been experimenting with the context features of 1.8 for github.com/jackc/pgx and have run across some surprising behavior.

When a context is canceled while a query is reading rows I observe the following behavior:

• rows.Err() is always <nil> even though query was interrupted. How do I detect a partial read of result set?
• Undefined number of additional rows read after context cancellation.
• Scan may or may not fail (this may be reasonable behavior).

I was able to reproduce this on Go 1.8rc3 with both pgx and pq.

Test case: https://github.com/jackc/context-rows-cancel

Is this expected behavior? Inability to detect an interrupted read would be undesirable. I'd also consider the extra rows read after cancellation to be undesirable, but not so serious.

Jack

--

You received this message because you are subscribed to the Google Groups "golang-sql" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-sql+...@googlegroups.com.
To post to this group, send email to golan...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-sql/3ba03d0a-a7bb-4d97-bb2a-bc4ad0a9e0e5%40googlegroups.com.

[Daniel Theophanes]

Hi Jack,

Thanks for working on this.

 

I'm working on context support for pgx. This test was partially to flesh out what desired/correct behavior. I still think that inability to detect partial read is a pretty serious deficiency regardless of underlying driver behavior.

I agree with this. I'm sorry I did not look at your posted issue the first time and assumed it was something else. I'll will file a go issue unless you file one first. I'm going to see if I can get this fixed ASAP.

Thanks, -Daniel

 

As far as correct driver behavior, is the correct response to a context cancellation to return ctx.Err() from whatever operation is interrupted?

Jack

Thanks, -Daniel

On Saturday, February 4, 2017 at 7:05:12 PM UTC-8, jack wrote:

I've been experimenting with the context features of 1.8 for github.com/jackc/pgx and have run across some surprising behavior.

When a context is canceled while a query is reading rows I observe the following behavior:

• rows.Err() is always <nil> even though query was interrupted. How do I detect a partial read of result set?
• Undefined number of additional rows read after context cancellation.
• Scan may or may not fail (this may be reasonable behavior).

I was able to reproduce this on Go 1.8rc3 with both pgx and pq.

Test case: https://github.com/jackc/context-rows-cancel

Is this expected behavior? Inability to detect an interrupted read would be undesirable. I'd also consider the extra rows read after cancellation to be undesirable, but not so serious.

Jack

--
You received this message because you are subscribed to the Google Groups "golang-sql" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-sql+...@googlegroups.com.
To post to this group, send email to golan...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-sql/3ba03d0a-a7bb-4d97-bb2a-bc4ad0a9e0e5%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "golang-sql" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-sql+...@googlegroups.com.
To post to this group, send email to golan...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-sql/58988F96.4000501%40jackchristensen.com.

[Daniel Theophanes]

I filled https://github.com/golang/go/issues/18961 . I hope to finish a CL for this today or tomorrow.

If at all possible, continue testing and implementing context this week. I'd like more testing while it is still possible to issue a fix in go1.8.

[Daniel Theophanes]

This change has landed and will be in go1.8. Another new bugfix will also be in go1.8.

I appreciate your attention to this Jack. After you update to the latest go1.8 in the release-branch-go1.8 in the git repo, I'd love to know of any other issue or concern you may have.

Thanks, -Daniel

On Monday, February 6, 2017 at 8:45:17 AM UTC-8, Daniel Theophanes wrote:

I filled https://github.com/golang/go/issues/18961 . I hope to finish a CL for this today or tomorrow.

If at all possible, continue testing and implementing context this week. I'd like more testing while it is still possible to issue a fix in go1.8.

On Mon, Feb 6, 2017 at 7:40 AM Daniel Theophanes <kard...@gmail.com> wrote:

Hi Jack,

Thanks for working on this.

 

I'm working on context support for pgx. This test was partially to flesh out what desired/correct behavior. I still think that inability to detect partial read is a pretty serious deficiency regardless of underlying driver behavior.

I agree with this. I'm sorry I did not look at your posted issue the first time and assumed it was something else. I'll will file a go issue unless you file one first. I'm going to see if I can get this fixed ASAP.

Thanks, -Daniel

As far as correct driver behavior, is the correct response to a context cancellation to return ctx.Err() from whatever operation is interrupted?

Jack

Thanks, -Daniel

On Saturday, February 4, 2017 at 7:05:12 PM UTC-8, jack wrote:

I've been experimenting with the context features of 1.8 for github.com/jackc/pgx and have run across some surprising behavior.

When a context is canceled while a query is reading rows I observe the following behavior:

• rows.Err() is always <nil> even though query was interrupted. How do I detect a partial read of result set?
• Undefined number of additional rows read after context cancellation.
• Scan may or may not fail (this may be reasonable behavior).

I was able to reproduce this on Go 1.8rc3 with both pgx and pq.

Test case: https://github.com/jackc/context-rows-cancel

Is this expected behavior? Inability to detect an interrupted read would be undesirable. I'd also consider the extra rows read after cancellation to be undesirable, but not so serious.

Jack

--
You received this message because you are subscribed to the Google Groups "golang-sql" group.

To unsubscribe from this group and stop receiving emails from it, send an email to golang-sql+unsubscribe@googlegroups.com.

To post to this group, send email to golan...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-sql/3ba03d0a-a7bb-4d97-bb2a-bc4ad0a9e0e5%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "golang-sql" group.

To unsubscribe from this group and stop receiving emails from it, send an email to golang-sql+unsubscribe@googlegroups.com.

[Jack Christensen]

On 02/08/2017 11:57 PM, Daniel Theophanes wrote:

This change has landed and will be in go1.8. Another new bugfix will also be in go1.8.

I appreciate your attention to this Jack. After you update to the latest go1.8 in the release-branch-go1.8 in the git repo, I'd love to know of any other issue or concern you may have.

Looks good so far! Thanks for getting this fixed.

Jack