Skip to content

queue: fix flaky Example_connectionPool #222

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

Merged
merged 3 commits into from
Oct 31, 2022

Conversation

oleg-jukovec
Copy link
Collaborator

The flaky test output:

=== RUN   Example_connectionPool
--- FAIL: Example_connectionPool (1.01s)
got:
Master 127.0.0.1:3014 is ready to work!
A Queue object is ready to work.
Send data: test_data
Master 127.0.0.1:3015 is ready to work!
task.Data() == nil
want:
Master 127.0.0.1:3014 is ready to work!
A Queue object is ready to work.
Send data: test_data
Master 127.0.0.1:3015 is ready to work!
Got data: test_data

We get a nil task if a ConnectionPool does not yet detect master->replica role switch. We need to add additional check for the case to make the test output deterministic.

@oleg-jukovec oleg-jukovec marked this pull request as ready for review October 7, 2022 10:05
Copy link
Member

@DifferentialOrange DifferentialOrange left a comment

Choose a reason for hiding this comment

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

LGTM after rebasing on master

@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/fix-flaky-queue-pool branch from c2fabab to fbed220 Compare October 10, 2022 07:11
@oleg-jukovec oleg-jukovec requested review from vr009 and removed request for AnaNek October 12, 2022 09:11
The patch fixes the return an invalid task object from queue.Take().
It may happen if an encoded task data has a nil value [1].

After the fix queue.Take() returns a nil value in the case.

1. https://github.com/msgpack/msgpack/blob/master/spec.md#nil-format
We get a nil task if a ConnectionPool does not yet detect
master->replica role switch. We need to add additional check for the
case to make the test output deterministic.
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/fix-flaky-queue-pool branch from fbed220 to 24a1c4e Compare October 31, 2022 10:40
It's better to wait for a discovered event instead of deactivated
to handle finish of a role update.
@oleg-jukovec oleg-jukovec merged commit 1cb4e0b into master Oct 31, 2022
@oleg-jukovec oleg-jukovec deleted the oleg-jukovec/fix-flaky-queue-pool branch October 31, 2022 11:08
@oleg-jukovec oleg-jukovec mentioned this pull request Oct 31, 2022
oleg-jukovec added a commit that referenced this pull request Oct 31, 2022
Overview

    The release adds support for the latest version of the
    queue package with master-replica switching.

Breaking changes

    There are no breaking changes in the release.

New features

    Support the queue 1.2.1 (#177).

    ConnectionHandler interface for handling changes of connections in
    ConnectionPool (#178).

    Execute, ExecuteTyped and ExecuteAsync methods to
    ConnectionPool (#176).

    ConnectorAdapter type to use ConnectionPool as Connector
    interface (#176).

    An example how to use queue and connection_pool subpackages
    together (#176).

Bugfixes

    Mode type description in the connection_pool subpackage (#208).

    Missed Role type constants in the connection_pool
    subpackage (#208).

    ConnectionPool does not close UnknownRole connections (#208).

    Segmentation faults in ConnectionPool requests after
    disconnect (#208).

    Addresses in ConnectionPool may be changed from an external
    code (#208).

    ConnectionPool recreates connections too often (#208).

    A connection is still opened after ConnectionPool.Close() (#208).

    Future.GetTyped() after Future.Get() does not decode response
    correctly (#213).

    Decimal package use a test function GetNumberLength instead of a
    package-level function getNumberLength (#219).

    Datetime location after encode + decode is unequal (#217).

    Wrong interval arithmetic with timezones (#221).

    Invalid MsgPack if STREAM_ID > 127 (#224).

    queue.Take() returns an invalid task (#222).
oleg-jukovec added a commit that referenced this pull request Oct 31, 2022
Overview

    The release adds support for the latest version of the
    queue package with master-replica switching.

Breaking changes

    There are no breaking changes in the release.

New features

    Support the queue 1.2.1 (#177).

    ConnectionHandler interface for handling changes of connections in
    ConnectionPool (#178).

    Execute, ExecuteTyped and ExecuteAsync methods to
    ConnectionPool (#176).

    ConnectorAdapter type to use ConnectionPool as Connector
    interface (#176).

    An example how to use queue and connection_pool subpackages
    together (#176).

Bugfixes

    Mode type description in the connection_pool subpackage (#208).

    Missed Role type constants in the connection_pool
    subpackage (#208).

    ConnectionPool does not close UnknownRole connections (#208).

    Segmentation faults in ConnectionPool requests after
    disconnect (#208).

    Addresses in ConnectionPool may be changed from an external
    code (#208).

    ConnectionPool recreates connections too often (#208).

    A connection is still opened after ConnectionPool.Close() (#208).

    Future.GetTyped() after Future.Get() does not decode response
    correctly (#213).

    Decimal package use a test function GetNumberLength instead of a
    package-level function getNumberLength (#219).

    Datetime location after encode + decode is unequal (#217).

    Wrong interval arithmetic with timezones (#221).

    Invalid MsgPack if STREAM_ID > 127 (#224).

    queue.Take() returns an invalid task (#222).
oleg-jukovec added a commit that referenced this pull request Nov 2, 2022
Overview

    The release adds support for the latest version of the
    queue package with master-replica switching.

Breaking changes

    There are no breaking changes in the release.

New features

    Support the queue 1.2.1 (#177).

    ConnectionHandler interface for handling changes of connections in
    ConnectionPool (#178).

    Execute, ExecuteTyped and ExecuteAsync methods to
    ConnectionPool (#176).

    ConnectorAdapter type to use ConnectionPool as Connector
    interface (#176).

    An example how to use queue and connection_pool subpackages
    together (#176).

Bugfixes

    Mode type description in the connection_pool subpackage (#208).

    Missed Role type constants in the connection_pool
    subpackage (#208).

    ConnectionPool does not close UnknownRole connections (#208).

    Segmentation faults in ConnectionPool requests after
    disconnect (#208).

    Addresses in ConnectionPool may be changed from an external
    code (#208).

    ConnectionPool recreates connections too often (#208).

    A connection is still opened after ConnectionPool.Close() (#208).

    Future.GetTyped() after Future.Get() does not decode response
    correctly (#213).

    Decimal package use a test function GetNumberLength instead of a
    package-level function getNumberLength (#219).

    Datetime location after encode + decode is unequal (#217).

    Wrong interval arithmetic with timezones (#221).

    Invalid MsgPack if STREAM_ID > 127 (#224).

    queue.Take() returns an invalid task (#222).
oleg-jukovec added a commit that referenced this pull request Nov 2, 2022
Overview

    The release adds support for the latest version of the
    queue package with master-replica switching.

Breaking changes

    There are no breaking changes in the release.

New features

    Support the queue 1.2.1 (#177).

    ConnectionHandler interface for handling changes of connections in
    ConnectionPool (#178).

    Execute, ExecuteTyped and ExecuteAsync methods to
    ConnectionPool (#176).

    ConnectorAdapter type to use ConnectionPool as Connector
    interface (#176).

    An example how to use queue and connection_pool subpackages
    together (#176).

Bugfixes

    Mode type description in the connection_pool subpackage (#208).

    Missed Role type constants in the connection_pool
    subpackage (#208).

    ConnectionPool does not close UnknownRole connections (#208).

    Segmentation faults in ConnectionPool requests after
    disconnect (#208).

    Addresses in ConnectionPool may be changed from an external
    code (#208).

    ConnectionPool recreates connections too often (#208).

    A connection is still opened after ConnectionPool.Close() (#208).

    Future.GetTyped() after Future.Get() does not decode response
    correctly (#213).

    Decimal package use a test function GetNumberLength instead of a
    package-level function getNumberLength (#219).

    Datetime location after encode + decode is unequal (#217).

    Wrong interval arithmetic with timezones (#221).

    Invalid MsgPack if STREAM_ID > 127 (#224).

    queue.Take() returns an invalid task (#222).
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