Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-26 Thread via GitHub
afs merged PR #2395: URL: https://github.com/apache/jena/pull/2395 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: pr-unsubscr...@jena.apache.org For

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-26 Thread via GitHub
afs commented on PR #2395: URL: https://github.com/apache/jena/pull/2395#issuecomment-2079389568 Excellent. Let's merge this and do any refinement as and when time permits. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-26 Thread via GitHub
Aklakan commented on PR #2395: URL: https://github.com/apache/jena/pull/2395#issuecomment-2079138969 I removed the class TestQueryAFS and moved the methods to TestQueryExecutionCancel. Are you going to adapt the code from volatile to AtomicReference? -- This is an automated

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-26 Thread via GitHub
Aklakan commented on code in PR #2395: URL: https://github.com/apache/jena/pull/2395#discussion_r1580848489 ## jena-arq/src/test/java/org/apache/jena/sparql/api/TestCancelQueryAFS.java: ## @@ -0,0 +1,155 @@ +package org.apache.jena.sparql.api; + +/* + * Copyright (c) Telicent

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-26 Thread via GitHub
Aklakan commented on code in PR #2395: URL: https://github.com/apache/jena/pull/2395#discussion_r1580848049 ## jena-arq/src/test/java/org/apache/jena/sparql/api/TestCancelQueryAFS.java: ## @@ -0,0 +1,155 @@ +package org.apache.jena.sparql.api; + +/* + * Copyright (c) Telicent

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-25 Thread via GitHub
afs commented on code in PR #2395: URL: https://github.com/apache/jena/pull/2395#discussion_r1579525377 ## jena-arq/src/test/java/org/apache/jena/sparql/api/TestCancelQueryAFS.java: ## @@ -0,0 +1,155 @@ +package org.apache.jena.sparql.api; + +/* + * Copyright (c) Telicent Ltd.

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-25 Thread via GitHub
afs commented on PR #2395: URL: https://github.com/apache/jena/pull/2395#issuecomment-2077253529 Looks good. I'll do some clearup - like not call a test class "... AFS". I've been looking at the Java memory model recently and there a volatile which could at least be an

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-21 Thread via GitHub
Aklakan commented on code in PR #2395: URL: https://github.com/apache/jena/pull/2395#discussion_r1573891140 ## jena-arq/src/test/java/org/apache/jena/sparql/api/TestCancelQueryAFS.java: ## @@ -0,0 +1,155 @@ +package org.apache.jena.sparql.api; + +/* + * Copyright (c) Telicent

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-21 Thread via GitHub
Aklakan commented on code in PR #2395: URL: https://github.com/apache/jena/pull/2395#discussion_r1573891202 ## jena-arq/src/test/java/org/apache/jena/sparql/api/TestCancelQueryAFS.java: ## @@ -0,0 +1,155 @@ +package org.apache.jena.sparql.api; + +/* + * Copyright (c) Telicent

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-21 Thread via GitHub
Aklakan commented on code in PR #2395: URL: https://github.com/apache/jena/pull/2395#discussion_r1573891140 ## jena-arq/src/test/java/org/apache/jena/sparql/api/TestCancelQueryAFS.java: ## @@ -0,0 +1,155 @@ +package org.apache.jena.sparql.api; + +/* + * Copyright (c) Telicent

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-21 Thread via GitHub
Aklakan commented on code in PR #2395: URL: https://github.com/apache/jena/pull/2395#discussion_r1573746863 ## jena-arq/src/main/java/org/apache/jena/sparql/exec/QueryExecDataset.java: ## @@ -89,6 +89,7 @@ public class QueryExecDataset implements QueryExec private long

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-21 Thread via GitHub
Aklakan commented on PR #2395: URL: https://github.com/apache/jena/pull/2395#issuecomment-2068029396 > I'm personally in favour of consistency (my perceived idea of consistency!) which is fail ASAP. Right, I agree that fail fast is typically easier to debug. I noticed that

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-17 Thread via GitHub
afs commented on PR #2395: URL: https://github.com/apache/jena/pull/2395#issuecomment-2061533423 Thanks for pointing out that failing at `select()` is a visible change. I'm personally in favour of consistency (my perceived idea of consistency!) which is fail ASAP. The `QueryExec` has

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-17 Thread via GitHub
afs commented on code in PR #2395: URL: https://github.com/apache/jena/pull/2395#discussion_r1569021399 ## jena-arq/src/main/java/org/apache/jena/sparql/exec/QueryExecDataset.java: ## @@ -89,6 +89,7 @@ public class QueryExecDataset implements QueryExec private long

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-16 Thread via GitHub
Aklakan commented on PR #2395: URL: https://github.com/apache/jena/pull/2395#issuecomment-2058981646 > So I'd say that the correct behavior Hm, "correctness" is the wrong wording - its an API contract question. The query exec methods that do not return an iterator are expected to

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-16 Thread via GitHub
Aklakan commented on PR #2395: URL: https://github.com/apache/jena/pull/2395#issuecomment-2058504184 test_cancel_select_1 does not raise the exception because the iterator is not accessed - the cancel flag is set though. IMO QueryCancelledException should only be thrown on iterator

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-15 Thread via GitHub
afs commented on PR #2395: URL: https://github.com/apache/jena/pull/2395#issuecomment-2057054016 Improving this is good to have. To try my understanding out, I wrote some tests. Maybe I'm misunderstanding the intention. Whole file:

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-14 Thread via GitHub
Aklakan commented on PR #2395: URL: https://github.com/apache/jena/pull/2395#issuecomment-2054133455 I added another test case for concurrent abort which should (occasionally) hang prior to this patch - the reason it should hang is that sometimes abort happens to be called before the

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-08 Thread via GitHub
Aklakan commented on PR #2395: URL: https://github.com/apache/jena/pull/2395#issuecomment-2042678206 @namedgraph Jena's machinery closes iterators upon requesting the (computation of the) next element of an aborted iterator. However, I have not yet come across your issue related to

Re: [PR] Fix for query exec when abort is called before exec [jena]

2024-04-08 Thread via GitHub
namedgraph commented on PR #2395: URL: https://github.com/apache/jena/pull/2395#issuecomment-2042040255 Reminds me of this issue I had but I never got around to isolating it... https://www.mail-archive.com/users@jena.apache.org/msg20564.html -- This is an automated message from the