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.

Sign up for GitHub

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

Jump to bottom

DATA RACE #218

Closed

alrsorokin opened this issue Sep 21, 2022 · 1 comment · Fixed by #258

Assignees

Labels

bug

alrsorokin commented Sep 21, 2022

go: 1.19
tarantool: 1.10 / 2.10

Все запускается из докера
Использую базовый образ для приложения (acim/go-reflex)
При запуске приложения получаем WARNING: DATA RACE
Пример приложения(warning) и лог:
warning.zip
tnt.log

oleg-jukovec added bug 2sp labels

Collaborator

oleg-jukovec commented Sep 22, 2022 •

edited

Loading

Thank you for the issue!

It does not look like a critical problem. The data race should not cause a problem because the ping request does not use a schema.

But in any case, we will try to fix it.

oleg-jukovec self-assigned this

oleg-jukovec added a commit that referenced this issue


          code health: do not pass a schema to init requests

b8a203e

Initial requests should not use a schema because it has not been
loaded yet.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: load and usage schema data race

35939df

We need to block shards to avoid schema usage by concurrent requests.
Now it can be a ping request or a watch request so it does not look
critical. We don't expect many of this requests and such requests
do not use schema at all.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: read state data race

8709a93

We need to use an atomic method to read the atomic value.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: TestFutureSetStateRaceCondition data race

44cf220

A use-case from our code: a response per AppendPush(). It looks like
it's enough to change the test.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: ConnectionMulti update addresses data race

7d452ea

We need to update an addresses slice under a lock because
we use the slice in ConnectionMulti.getCurrentConnection().

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: ConnectionMulti.Close() data race

ffe84ee

We need to use an atomic method to update the atomic value.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: data race in poolWatcher.Unregister()

8b228ca

You need to protect the poolWatcher.unregistered variable to avoid
the data race. It does not look too critical because we don't
expect that performance of poolWatcher.Unregister() is matter
in cuncurrent calls case.

It could also lead to crashes.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: TestConnectionHandlerOpenUpdateClose data race

2a936d6

It is better to use atomic counters.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: TestConnectionHandlerUpdateError data race

001bf9c

It is better to use atomic counters.

Part of #218

oleg-jukovec added a commit that referenced this issue


          ci: add tests with data race detector

c5da299

Tests execution result may differ due to different timings, so it is
better to test together, rather than instead.

Closes #218

oleg-jukovec added a commit that referenced this issue


          bugfix: poolWatcher.Unregister() data race

59bad84

You need to protect the poolWatcher.unregistered variable to avoid
the data race. It does not look too critical because we don't
expect that performance of poolWatcher.Unregister() is matter
in cuncurrent calls case.

It could also lead to crashes.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: TestConnectionHandlerOpenUpdateClose data race

2e3ebd1

It is better to use atomic counters.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: TestConnectionHandlerUpdateError data race

3c28295

It is better to use atomic counters.

Part of #218

oleg-jukovec added a commit that referenced this issue


          ci: add tests with data race detector

a591c9a

Tests execution result may differ due to different timings, so it is
better to test together, rather than instead.

Closes #218

oleg-jukovec mentioned this issue

ci: add tests with data race detector #258

Merged

3 tasks

oleg-jukovec added a commit that referenced this issue


          code health: do not pass a schema to init requests

6e4da11

Initial requests should not use a schema because it has not been
loaded yet.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: load and usage schema data race

09912e2

We need to block shards to avoid schema usage by concurrent requests.
Now it can be a ping request or a watch request so it does not look
critical. We don't expect many of this requests and such requests
do not use schema at all.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: read state data race

ddaca41

We need to use an atomic method to read the atomic value.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: TestFutureSetStateRaceCondition data race

2a3f707

A use-case from our code: a response per AppendPush(). It looks like
it's enough to change the test.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: ConnectionMulti update addresses data race

7883b79

We need to update an addresses slice under a lock because
we use the slice in ConnectionMulti.getCurrentConnection().

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: ConnectionMulti.Close() data race

97b08be

We need to use an atomic method to update the atomic value.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: poolWatcher.Unregister() data race

3f173ba

You need to protect the poolWatcher.unregistered variable to avoid
the data race. It does not look too critical because we don't
expect that performance of poolWatcher.Unregister() is matter
in cuncurrent calls case.

It could also lead to crashes.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: TestConnectionHandlerOpenUpdateClose data race

b082357

It is better to use atomic counters.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: TestConnectionHandlerUpdateError data race

a7b987d

It is better to use atomic counters.

Part of #218

oleg-jukovec added a commit that referenced this issue


          ci: add tests with data race detector

05c26aa

Tests execution result may differ due to different timings, so it is
better to test together, rather than instead.

Closes #218

oleg-jukovec added a commit that referenced this issue


          bugfix: TestFutureSetStateRaceCondition data race

a1a7476

A use-case from our code: a response per AppendPush(). It looks like
it's enough to change the test.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: ConnectionMulti update addresses data race

08d4b1f

We need to update an addresses slice under a lock because
we use the slice in ConnectionMulti.getCurrentConnection().

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: ConnectionMulti.Close() data race

We need to use an atomic method to update the atomic value.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: poolWatcher.Unregister() data race

0d6e397

You need to protect the poolWatcher.unregistered variable to avoid
the data race. It does not look too critical because we don't
expect that performance of poolWatcher.Unregister() is matter
in cuncurrent calls case.

It could also lead to crashes.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: TestConnectionHandlerOpenUpdateClose data race

ae38068

It is better to use atomic counters.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: TestConnectionHandlerUpdateError data race

63f7f9a

It is better to use atomic counters.

Part of #218

oleg-jukovec added a commit that referenced this issue


          ci: add tests with data race detector

1f14554

Tests execution result may differ due to different timings, so it is
better to test together, rather than instead.

Closes #218

oleg-jukovec added a commit that referenced this issue


          bugfix: queue/Example_connectionPool data race

745b04f

Part of #218

oleg-jukovec added a commit that referenced this issue


          ci: add tests with data race detector

e05e0d8

Tests execution result may differ due to different timings, so it is
better to test together, rather than instead.

Closes #218

oleg-jukovec added a commit that referenced this issue


          bugfix: queue/Example_connectionPool data race

9ec8350

Part of #218

oleg-jukovec added a commit that referenced this issue


          ci: add tests with data race detector

ba53a95

Tests execution result may differ due to different timings, so it is
better to test together, rather than instead.

Closes #218

oleg-jukovec closed this as completed in #258

oleg-jukovec added a commit that referenced this issue


          code health: do not pass a schema to init requests

de7e60b

Initial requests should not use a schema because it has not been
loaded yet.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: load and usage schema data race

11b716d

We need to block shards to avoid schema usage by concurrent requests.
Now it can be a ping request or a watch request so it does not look
critical. We don't expect many of this requests and such requests
do not use schema at all.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: read state data race

d7974ea

We need to use an atomic method to read the atomic value.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: TestFutureSetStateRaceCondition data race

ec7e2b3

A use-case from our code: a response per AppendPush(). It looks like
it's enough to change the test.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: ConnectionMulti update addresses data race

444509c

We need to update an addresses slice under a lock because
we use the slice in ConnectionMulti.getCurrentConnection().

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: ConnectionMulti.Close() data race

a3b5b97

We need to use an atomic method to update the atomic value.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: poolWatcher.Unregister() data race

d8ca1c1

You need to protect the poolWatcher.unregistered variable to avoid
the data race. It does not look too critical because we don't
expect that performance of poolWatcher.Unregister() is matter
in cuncurrent calls case.

It could also lead to crashes.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: TestConnectionHandlerOpenUpdateClose data race

6a51ef1

It is better to use atomic counters.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: TestConnectionHandlerUpdateError data race

bda241e

It is better to use atomic counters.

Part of #218

oleg-jukovec added a commit that referenced this issue


          bugfix: queue/Example_connectionPool data race

81a613d

Part of #218

oleg-jukovec added a commit that referenced this issue


          ci: add tests with data race detector

f300bd9

Tests execution result may differ due to different timings, so it is
better to test together, rather than instead.

Closes #218

oleg-jukovec added a commit that referenced this issue


          Release 1.11.0

8c7f9ef

Overview

    The release adds pagination support and wrappers for for the
    crud module.

Breaking changes

    There are no breaking changes in the release.

New features

    Support pagination (#246).

    A Makefile target to test with race detector (#218).

    Support CRUD API (#108).

    An ability to replace a base network connection to a Tarantool
    instance (#265).

Bugfixes

    Several non-critical data race issues (#218).

    ConnectionPool does not properly handle disconnection with
    Opts.Reconnect set (#272).

oleg-jukovec mentioned this issue

Release 1.11.0 #279

Merged

oleg-jukovec added a commit that referenced this issue


          Release 1.11.0

Overview

    The release adds pagination support and wrappers for the
    crud module.

Breaking changes

    There are no breaking changes in the release.

New features

    Support pagination (#246).

    A Makefile target to test with race detector (#218).

    Support CRUD API (#108).

    An ability to replace a base network connection to a Tarantool
    instance (#265).

Bugfixes

    Several non-critical data race issues (#218).

    ConnectionPool does not properly handle disconnection with
    Opts.Reconnect set (#272).

oleg-jukovec added a commit that referenced this issue


          Release 1.11.0

00da9dc

Overview

    The release adds pagination support and wrappers for the
    crud module.

Breaking changes

    There are no breaking changes in the release.

New features

    Support pagination (#246).

    A Makefile target to test with race detector (#218).

    Support CRUD API (#108).

    An ability to replace a base network connection to a Tarantool
    instance (#265).

Bugfixes

    Several non-critical data race issues (#218).

    Build on Apple M1 with OpenSSL (#260).

    ConnectionPool does not properly handle disconnection with
    Opts.Reconnect set (#272).

oleg-jukovec added a commit that referenced this issue


          Release 1.11.0

084c48d

Overview

    The release adds pagination support and wrappers for the
    crud module.

Breaking changes

    There are no breaking changes in the release.

New features

    Support pagination (#246).

    A Makefile target to test with race detector (#218).

    Support CRUD API (#108).

    An ability to replace a base network connection to a Tarantool
    instance (#265).

    Missed iterator constant (#285).

Bugfixes

    Several non-critical data race issues (#218).

    Build on Apple M1 with OpenSSL (#260).

    ConnectionPool does not properly handle disconnection with
    Opts.Reconnect set (#272).

oleg-jukovec added a commit that referenced this issue


          Release 1.11.0

ab61bad

Overview

    The release adds pagination support and wrappers for the
    crud module.

Breaking changes

    There are no breaking changes in the release.

New features

    Support pagination (#246).

    A Makefile target to test with race detector (#218).

    Support CRUD API (#108).

    An ability to replace a base network connection to a Tarantool
    instance (#265).

    Missed iterator constant (#285).

Bugfixes

    Several non-critical data race issues (#218).

    Build on Apple M1 with OpenSSL (#260).

    ConnectionPool does not properly handle disconnection with
    Opts.Reconnect set (#272).

    Watcher events loss with a small per-request timeout (#284).

    Connect() panics on concurrent schema update (#278).

    Wrong Ttr setup by Queue.Cfg() (#278).

    Flaky queue/Example_connectionPool (#278).

    Flaky queue/Example_simpleQueueCustomMsgPack (#277).

Other

    queue module version bumped to 1.3.0 (#278).

oleg-jukovec added a commit that referenced this issue


          Release 1.11.0

cdfcddf

Overview

    The release adds pagination support and wrappers for the
    crud module.

Breaking changes

    There are no breaking changes in the release.

New features

    Support pagination (#246).

    A Makefile target to test with race detector (#218).

    Support CRUD API (#108).

    An ability to replace a base network connection to a Tarantool
    instance (#265).

    Missed iterator constant (#285).

Bugfixes

    Several non-critical data race issues (#218).

    Build on Apple M1 with OpenSSL (#260).

    ConnectionPool does not properly handle disconnection with
    Opts.Reconnect set (#272).

    Watcher events loss with a small per-request timeout (#284).

    Connect() panics on concurrent schema update (#278).

    Wrong Ttr setup by Queue.Cfg() (#278).

    Flaky queue/Example_connectionPool (#278).

    Flaky queue/Example_simpleQueueCustomMsgPack (#277).

Other

    queue module version bumped to 1.3.0 (#278).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment