Baby tsakhes/gh 502 flaky test connection handler open update close#15
Open
babyTsakhes wants to merge 645 commits into
Open
Conversation
Use skipIfLess helpers like in other test conditions. Follows #119
In Tarantool, user space ids starts from 512. You can set arbitrary id or use autoincrement (sequence also starts from 512). Unfortunately, mixing spaces with autoincremented ids and spaces with explicit ids may cause id conflict [1]. Since there are cases when we cannot explicitly set space id (creating a space with SQL), a short range of free ids (from 512 to 515) causes problems. This patch increases range of free ids (now it's from 512 to 615) so it should be ok until [1] is resolved. 1. tarantool/tarantool#8036 Part of #215
Set up package paths so queue test instances could be run with arbitrary work_dir. Part of #215
Generate tmp work directories for each new Tarantool instance, if not specified. It prevents possible directory clash issues. Later `ioutil.TempDir` (deprecated since Go 1.17) must be replaced with `os.MkdirTemp` (introduced in Go 1.16 as a replacement) when we drop support of Go 1.16 an older. Part of #215
Make skip helpers public. Add more wrappers to reduce code duplication. Part of #215
Support session settings in a separate subpackage "settings" [1]. It allows to create a specific requests to get or set session settings. Settings are independent for each connection. 1. https://www.tarantool.io/en/doc/latest/reference/reference_lua/box_space/_session_settings/ Closes #215
We need to lock mutexes everywhere in the same order to avoid the deadlock: 1. Container with watchers. 2. Watcher.
The authentication method works only with Tarantool EE [1]. 1. tarantool/enterprise_doc#206 Closes #243
The patch adds a second golangci-lint run that prints errors in human-readable format [1]. 1. golangci/golangci-lint-action#119 Closes #231
We don't need to over-optimize the work with requests with contexts that already done. It doesn't look like a use case. It is better to simplify the code. Part of #244
The patch makes the test more deterministic. It helps to avoid canceling a request with an already received response. Closes #244
After the fix the test stops tarantool intances to avoid concurrent reconnects by a ConnectionMulti instance. Closes #234
After this patch, "make test" behaves similar to "make test-*". Part of #214
Before this patch, calling StopTarantool wasn't idempotent because it accepts a struct copy and doesn't actually set Cmd to nil. Setting Cmd.Process to nil is effective since it's a pointer. Reworking helpers to use pointer would be better, but it would break existing API.
If connected to Tarantool 2.10 or newer, after this patch a connection supports server graceful shutdown [1]. In this case, server will wait until all client requests will be finished and client disconnects before going down (server also may go down by timeout). Client reconnect will happen if connection options enable reconnect. Beware that graceful shutdown event initialization is asynchronous. 1. https://www.tarantool.io/en/doc/latest/dev_guide/internals/iproto/graceful_shutdown/ Closes #214
Overview
The release improves compatibility with new Tarantool versions.
Breaking changes
There are no breaking changes in the release.
New features
Support iproto feature discovery (#120).
Support errors extended information (#209).
Support error type in MessagePack (#209).
Support event subscription (#119).
Support session settings (#215).
Support pap-sha256 authorization method (Tarantool EE
feature) (#243).
Support graceful shutdown (#214).
Bugfixes
Decimal package uses a test variable DecimalPrecision instead
of a package-level variable decimalPrecision (#233).
Flaky test TestClientRequestObjectsWithContext (#244).
Flaky test multi/TestDisconnectAll (#234).
Technically, the connector can now work with Tarantool 1.6+. But we decided to simplify our CI and do not declare support for older versions [1]. 1. #198
A user could fetch a position of a last tuple using a new method
of the SelectRequest type:
selectRequest = selectRequest.FetchPos(true)
The position will be stored in a new field of the Response type:
Response.Pos
A user could specify a tuple from which selection must continue or
its position with a new method of the SelectRequest type:
selectRequest = selectRequest.After([]interface{}{23})
or
selectRequest = selectRequest.After(resp.Pos)
In action it looks like:
req := NewSelectRequest(space).Key(key).Limit(10).FetchPos(true)
for condition {
resp, err := conn.Do(req).Get()
// ...
req = req.After(resp.Pos)
}
1. tarantool/tarantool#7639
Part of #246
Initial requests should not use a schema because it has not been loaded yet. Part of #218
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
We need to use an atomic method to read the atomic value. Part of #218
A use-case from our code: a response per AppendPush(). It looks like it's enough to change the test. Part of #218
We need to update an addresses slice under a lock because we use the slice in ConnectionMulti.getCurrentConnection(). Part of #218
We need to use an atomic method to update the atomic value. Part of #218
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
It is better to use atomic counters. Part of #218
Returing wrapped context.Cause(ctx) error in <-ctx.Done() case. Allows compare errors using errors.Is/As. Add 3 tests errors checking comparabily. Fixes #457
The greeting flags of functions IPROTO_FEATURE_IS_SYNC and IPROTO_FEATURE_INSERT_ARROW protocol function flags were added. Closes #466
Removed deprecated pool's methods. Related tests are updated (deleted or refactored). Also removed mentioned methods in interfaces declarations. Fixes #478
Existing benchmarks are no longer relevant - no one knows what they're testing. Instead, benchmarks have been added for the following metrics: * Number of allocations. * RPS on synchronous and asynchronous modes.
It should help to fix flaky tests on CI.
It seems that a connection's file descriptor is not always fully ready after the connection is created. The hack helps to avoid the "bad file descriptor" flaky errors for the test.
Removed `box.session.push()` support: * `Future.AppendPush()`, `Future.GetIterator()` methods; * `ResponseIterator` and `TimeoutResponseIterator` types; * `pushes[]` field in `Future` and related methods. Connection.reader() logs message about `box.session.push` is unsupported, when it reads IPROTO_CHUNK. Closes #480
Removed unused Future.ready channel. Closes #497
Removed deprecated `Connection` methods. Related tests are updated (deleted or refactored). Closes #479
Tests and methods are updated with enabling `errcheck` for `golangci-lint` check. Closes #334
babyTsakhes
force-pushed
the
babyTsakhes/gh-502-flaky_TestConnectionHandlerOpenUpdateClose
branch
from
1dd596b to
6f4180e
Compare
The welcome flags for the IPROTO_FEATURE_IS_SYNC functions have been added and IPROTO_FEATURE_INSERT_ARROW for IPROTO_FEATURE_INSERT_ARROW protocol functions. Changed it CHANGELOG.md in the previous version, it was in the wrong version. Changed #466
- Remove: OptUint, OptInt, OptFloat64, OptString, OptBool, OptTuple. - Remove: MakeOptUint, MakeOptInt, MakeOptFloat64, MakeOptString, MakeOptBool, MakeOptTuple. - Update: All option structs to use option.* types. - Add type Any from go-option. - Fix: Test type inconsistencies. Closes #492
This commit reduces allocations. Future.done allocation replaced with - Future.cond (sync.Cond) - Future.finished (bool) Other code use `Future.finished` instead `Future.done == nil` check. Added Future.finish() marks Future as done. Future.WaitChan() now creates channel on demand. Closes #496
babyTsakhes
force-pushed
the
babyTsakhes/gh-502-flaky_TestConnectionHandlerOpenUpdateClose
branch
from
6f4180e to
a0bc666
Compare
Fixing mistakes in previous `errcheck` commit. Closes #514
babyTsakhes
force-pushed
the
babyTsakhes/gh-502-flaky_TestConnectionHandlerOpenUpdateClose
branch
3 times, most recently
from
cdc571e to
44195e6
Compare
Fixed box/testdata/config.lua. Now, tests follow new Tarantool behavior: the 'super' roles now contains the 'grant' previlege. So we need to set a list of privileges directly instead of the 'super' role usage. Closes #511 Co-authored-by: Oleg Jukovec <oleg.jukovec@gmail.com>
It looks like the implementation of Tarantool changed and we need to update the test.
This commit bumps the Ubuntu distro version up to 24.04 (noble) in reusable_testing workflow, since Ubuntu 22.04 (jammy) leads to dpkg conflicts in the integrational testing with Tarantool. Needed for tarantool/tarantool#11220
babyTsakhes
force-pushed
the
babyTsakhes/gh-502-flaky_TestConnectionHandlerOpenUpdateClose
branch
3 times, most recently
from
e8283d2 to
fbdbc18
Compare
oleg-jukovec
force-pushed
the
babyTsakhes/gh-502-flaky_TestConnectionHandlerOpenUpdateClose
branch
from
fbdbc18 to
adba830
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.