Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(httphandler): req should return as soon as results is exhausted. #5462

Merged
merged 5 commits into from
May 20, 2022

Conversation

youngsofun
Copy link
Member

@youngsofun youngsofun commented May 19, 2022

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

Summary about this PR

  1. carry bool done aside block when pop()
  2. notify when stop_push()
  3. not return data and next_url when error.

Changelog

  • Bug Fix

Related Issues

Fixes #5456

@vercel
Copy link

vercel bot commented May 19, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
databend ⬜️ Ignored (Inspect) May 19, 2022 at 3:49PM (UTC)

@mergify
Copy link
Contributor

mergify bot commented May 19, 2022

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

@mergify mergify bot added the pr-bugfix this PR patches a bug in codebase label May 19, 2022
@youngsofun youngsofun requested a review from junnplus May 19, 2022 08:37
@BohuTANG BohuTANG removed their request for review May 19, 2022 08:39
@BohuTANG
Copy link
Member

failures:
servers::http::http_query_handlers::test_pagination always failed.

https://github.com/datafuselabs/databend/runs/6505375747?check_suite_focus=true#step:3:3320

@youngsofun
Copy link
Member Author

@BohuTANG should have been fixed. took a while to debug.

@youngsofun
Copy link
Member Author

youngsofun commented May 19, 2022

@TCeason need your help

02_0000_kill_query:                                                     2022-05-19T12:23:26.361052Z ERROR databend_query::pipelines::processors::processor_merge: Merge processor cannot push data: channel closed
[ FAIL ] - return code 1
, result:

Traceback (most recent call last):
  File "/home/runner/work/databend/databend/tests/helpers/uexpect.py", line 178, in read
    data += self.queue.get(timeout=timeleft)
  File "/usr/lib/python3.8/queue.py", line 178, in get
    raise Empty
_queue.Empty

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/runner/work/databend/databend/tests/helpers/uexpect.py", line 152, in expect
    data = self.read(timeout=timeleft, raise_exception=True)
  File "/home/runner/work/databend/databend/tests/helpers/uexpect.py", line 186, in read
    raise TimeoutError(timeout)
uexpect.TimeoutError: Timeout 60.000s

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/runner/work/databend/databend/tests/suites/1_stateful/02_query/02_0000_kill_query.py", line 25, in <module>
    with NativeClient(name='client2>') as client2:
  File "/home/runner/work/databend/databend/tests/suites/1_stateful/02_query/../../../helpers/native_client.py", line 38, in __init__
    self.client.expect('[#\$] ', timeout=60)
  File "/home/runner/work/databend/databend/tests/helpers/uexpect.py", line 159, in expect
    raise exception
uexpect.ExpectTimeoutError: Timeout 60.000s for '[#\\$] ' 

@TCeason
Copy link
Collaborator

TCeason commented May 19, 2022

@TCeason need your help

02_0000_kill_query:                                                     2022-05-19T12:23:26.361052Z ERROR databend_query::pipelines::processors::processor_merge: Merge processor cannot push data: channel closed
[ FAIL ] - return code 1
, result:

Traceback (most recent call last):
  File "/home/runner/work/databend/databend/tests/helpers/uexpect.py", line 178, in read
    data += self.queue.get(timeout=timeleft)
  File "/usr/lib/python3.8/queue.py", line 178, in get
    raise Empty
_queue.Empty

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/runner/work/databend/databend/tests/helpers/uexpect.py", line 152, in expect
    data = self.read(timeout=timeleft, raise_exception=True)
  File "/home/runner/work/databend/databend/tests/helpers/uexpect.py", line 186, in read
    raise TimeoutError(timeout)
uexpect.TimeoutError: Timeout 60.000s

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/runner/work/databend/databend/tests/suites/1_stateful/02_query/02_0000_kill_query.py", line 25, in <module>
    with NativeClient(name='client2>') as client2:
  File "/home/runner/work/databend/databend/tests/suites/1_stateful/02_query/../../../helpers/native_client.py", line 38, in __init__
    self.client.expect('[#\$] ', timeout=60)
  File "/home/runner/work/databend/databend/tests/helpers/uexpect.py", line 159, in expect
    raise exception
uexpect.ExpectTimeoutError: Timeout 60.000s for '[#\\$] ' 

Yes that a flaky test, needs a rerun.

@BohuTANG
Copy link
Member

@TCeason need your help

02_0000_kill_query:                                                     2022-05-19T12:23:26.361052Z ERROR databend_query::pipelines::processors::processor_merge: Merge processor cannot push data: channel closed
[ FAIL ] - return code 1
, result:

Traceback (most recent call last):
  File "/home/runner/work/databend/databend/tests/helpers/uexpect.py", line 178, in read
    data += self.queue.get(timeout=timeleft)
  File "/usr/lib/python3.8/queue.py", line 178, in get
    raise Empty
_queue.Empty

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/runner/work/databend/databend/tests/helpers/uexpect.py", line 152, in expect
    data = self.read(timeout=timeleft, raise_exception=True)
  File "/home/runner/work/databend/databend/tests/helpers/uexpect.py", line 186, in read
    raise TimeoutError(timeout)
uexpect.TimeoutError: Timeout 60.000s

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/runner/work/databend/databend/tests/suites/1_stateful/02_query/02_0000_kill_query.py", line 25, in <module>
    with NativeClient(name='client2>') as client2:
  File "/home/runner/work/databend/databend/tests/suites/1_stateful/02_query/../../../helpers/native_client.py", line 38, in __init__
    self.client.expect('[#\$] ', timeout=60)
  File "/home/runner/work/databend/databend/tests/helpers/uexpect.py", line 159, in expect
    raise exception
uexpect.ExpectTimeoutError: Timeout 60.000s for '[#\\$] ' 

Yes that a flaky test, needs a rerun.

Re-run can't save it. It seems to fail every time, needs improvement, or skip this test.

@youngsofun
Copy link
Member Author

youngsofun commented May 19, 2022

a unit test can not stop, I am working on it.

@TCeason
Copy link
Collaborator

TCeason commented May 19, 2022

@TCeason need your help

02_0000_kill_query:                                                     2022-05-19T12:23:26.361052Z ERROR databend_query::pipelines::processors::processor_merge: Merge processor cannot push data: channel closed
[ FAIL ] - return code 1
, result:

Traceback (most recent call last):
  File "/home/runner/work/databend/databend/tests/helpers/uexpect.py", line 178, in read
    data += self.queue.get(timeout=timeleft)
  File "/usr/lib/python3.8/queue.py", line 178, in get
    raise Empty
_queue.Empty

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/runner/work/databend/databend/tests/helpers/uexpect.py", line 152, in expect
    data = self.read(timeout=timeleft, raise_exception=True)
  File "/home/runner/work/databend/databend/tests/helpers/uexpect.py", line 186, in read
    raise TimeoutError(timeout)
uexpect.TimeoutError: Timeout 60.000s

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/runner/work/databend/databend/tests/suites/1_stateful/02_query/02_0000_kill_query.py", line 25, in <module>
    with NativeClient(name='client2>') as client2:
  File "/home/runner/work/databend/databend/tests/suites/1_stateful/02_query/../../../helpers/native_client.py", line 38, in __init__
    self.client.expect('[#\$] ', timeout=60)
  File "/home/runner/work/databend/databend/tests/helpers/uexpect.py", line 159, in expect
    raise exception
uexpect.ExpectTimeoutError: Timeout 60.000s for '[#\\$] ' 

Yes that a flaky test, needs a rerun.

Re-run can't save it. It seems to fail every time, needs improvement, or skip this test.

Got and doing

@TCeason
Copy link
Collaborator

TCeason commented May 19, 2022

And I think you can skip this test tmp. @youngsofun

remove file : 02_0000_kill_query.result in your pr.

@youngsofun
Copy link
Member Author

And I think you can skip this test tmp. @youngsofun

remove file : 02_0000_kill_query.result in your pr.

@TCeason
removed

@youngsofun
Copy link
Member Author

@junnplus rebased to rm 02_0000_kill_query.result

@youngsofun
Copy link
Member Author

youngsofun commented May 19, 2022

servers::http::http_query_handlers::test_http_session seems like a flaky test, can not reproduce on my mac
. seen for the first time and passed after rerun, maybe we can merge for now. I will try to improve it later,others can skip it if see it again

@BohuTANG BohuTANG merged commit 48bb67d into databendlabs:main May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need-review pr-bugfix this PR patches a bug in codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: http handler performance degradation
5 participants