Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

Support query execution timeout #64

Merged
merged 1 commit into from
Mar 7, 2018
Merged

Support query execution timeout #64

merged 1 commit into from
Mar 7, 2018

Conversation

Khatskevich
Copy link
Contributor

key points:

Closes #25

Copy link
Member

@Totktonada Totktonada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally LGTM, minor comments added.

@@ -864,6 +885,8 @@ function accessor_general.new(opts, funcs)
DEF_RESULTING_OBJECT_CNT_MAX
local fetched_object_cnt_max = opts.fetched_object_cnt_max or
DEF_FETCHED_OBJECT_CNT_MAX
-- todo: move this setting to `tgql.compile` after #59
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use TODO or XXX marks, they highlighted by vim.

fetched_object_cnt = 0
}
qcontext.deadline_clock = clock.monotonic64() +
settings.timeout_ms * 1000 * 1000
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indent is strange.

@@ -864,6 +885,8 @@ function accessor_general.new(opts, funcs)
DEF_RESULTING_OBJECT_CNT_MAX
local fetched_object_cnt_max = opts.fetched_object_cnt_max or
DEF_FETCHED_OBJECT_CNT_MAX
-- todo: move this setting to `tgql.compile` after #59
local timeout_ms = opts.timeout_ms or DEF_TIMEOUT_MS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should check user-provided input.

@Totktonada Totktonada requested a review from SudoBobo March 4, 2018 23:12
@Totktonada
Copy link
Member

Added @SudoBobo as optional reviewer.

key points:
 - use clock.monotonic64() because it is fast and it changes without
   yields
 - add option `timeout_ms` to accessor.new()
 - after #59 the option should be moved to `tgql.compile()`; see #63

Closes #25
@Totktonada Totktonada requested a review from Kasen March 5, 2018 16:11
@Khatskevich Khatskevich merged commit dea44b9 into master Mar 7, 2018
@Khatskevich Khatskevich deleted the kh/timeout branch March 9, 2018 09:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants