Skip to content

CXXCBC-588: Update timeout sent to server on each columnar query retry#654

Merged
DemetrisChr merged 1 commit intocouchbase:mainfrom
DemetrisChr:CXXCBC-588-col-retry-timeout
Sep 9, 2024
Merged

CXXCBC-588: Update timeout sent to server on each columnar query retry#654
DemetrisChr merged 1 commit intocouchbase:mainfrom
DemetrisChr:CXXCBC-588-col-retry-timeout

Conversation

@DemetrisChr
Copy link
Copy Markdown
Contributor

Changes

  • Update the timeout written in the query request payload when retrying, according to the time remaining until the deadline.
  • Fix a bug where not setting the timeout in the options (i.e. relying on the cluster-wide default) resulted in the query timing out immediately.
    • This is because the HTTP component's timeout would not be set, which resulted in the epoch being passed on to the http_session_manager's connect_then_send_pending_op as the deadline.
    • The timeout is now always sent to the HTTP component.
  • Remove the dispatch timeout test which was succeeding because of the bug above. Dispatching appears to succeed very quickly, so even with a timeout 1ms the test was not consistently passing - we probably need to find another way to test it.
  • Use the default timeouts in tests that don't need a specific timeout value (this was previously hiding the bug)

@DemetrisChr DemetrisChr marked this pull request as draft September 5, 2024 16:45
@DemetrisChr DemetrisChr force-pushed the CXXCBC-588-col-retry-timeout branch from 1d74ca7 to 3562231 Compare September 6, 2024 14:27
@DemetrisChr DemetrisChr force-pushed the CXXCBC-588-col-retry-timeout branch from 3562231 to 353f239 Compare September 6, 2024 14:46
@DemetrisChr DemetrisChr marked this pull request as ready for review September 6, 2024 14:56
Comment thread core/columnar/query_component.cxx
@avsej avsej closed this Sep 6, 2024
@DemetrisChr DemetrisChr reopened this Sep 6, 2024
Copy link
Copy Markdown
Contributor

@thejcfactor thejcfactor left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@DemetrisChr DemetrisChr merged commit 6b64c08 into couchbase:main Sep 9, 2024
@DemetrisChr DemetrisChr deleted the CXXCBC-588-col-retry-timeout branch September 9, 2024 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants