Skip to content
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

Support vshard names as keys #404

Merged
merged 6 commits into from
Dec 25, 2023

Conversation

DifferentialOrange
Copy link
Member

@DifferentialOrange DifferentialOrange commented Dec 22, 2023

In vshard 0.1.25, new feature related to vshard configuration and storage info was introduced [1]. If the new mode is used, crud module fails to bootstrap and work in several places. This feature is enabled by Tarantool 3.0 if vshard cluster was configured with 3.0 config.

After this patch, it is possible to bootstrap a vshard cluster with new configuration mode. We run all existing vshard tests with new modes after this patch. Module code was updated to support both possible mods.

This PR also introduces several minor fixes inspired by flaky tests. (They mostly have failed locally on my PC with vinyl engine, but it is possible to get the same result on Github Actions.)

I didn't forget about

  • Tests
  • Changelog

Closes #403

@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/gh-403-names-as-keys branch 3 times, most recently from fb0e564 to f484d5d Compare December 25, 2023 09:43
@DifferentialOrange DifferentialOrange marked this pull request as ready for review December 25, 2023 10:30
@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/gh-403-names-as-keys branch 2 times, most recently from c10e9cc to 39ca62d Compare December 25, 2023 12:29
Copy link
Contributor

@better0fdead better0fdead left a comment

Choose a reason for hiding this comment

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

Hi, thx for patch. Code looks amazing. I think it would be good to update README section about min and max operations. (add arguments description as it is done for other operations)
Also, pls fix grammar in commit messages:

  • 1st commit last sentence: Module code was updated to support both possible MODES.
  • 4th commit: Now it is possible to perform min/max on
    write instances, and TESTS were updated to do so.

In vshard 0.1.25, new feature related to vshard configuration and
storage info was introduced [1]. If the new mode is used, crud module
fails to bootstrap and work in several places. This feature is enabled
by Tarantool 3.0 if vshard cluster was configured with 3.0 config.

After this patch, it is possible to bootstrap a vshard cluster with
new configuration mode. We run all existing vshard tests with new modes
after this patch. Module code was updated to support both possible
modes.

1. tarantool/vshard#426

Closes #403
The following sequence of events happens:
- data inserted with crud,
- schema updated 12 times,
- data updated with crud.

For Tarantool supporting field name in updates, there is no additional
check for a field name presenting in current format. So it is possible
to get outdated metadata.

For some reason, new identification mode had uncovered this issue. It
is likely related to the change in the order of servers in maps.
For some reason, new vshard identification mode had uncovered flaky test
cases for read operations (99% of fails were with vinyl engine).
It is likely related to the change in the order of servers in maps.

Since we use async replication, it is possible that we perform select,
get, count or pairs operation over replica which hasn't received
new data yet. To get rid of assumption that replication is always faster
than our reading, this patch ensured that all read operations use master
unless they want to test reading from replicas.
This patch follows the previous one. To stabilize tests, we perform
read operations on masters. Now it is possible to perform min/max on
write instances, and tests were updated to do so.
Any test run runs perf tests as well -- to check that they work fine.
To make tests faster, we run them for 2-5 seconds, but it's still a lot
of time.
If Tarantool older than 2.8.1 [1] was used to update nullable fields,
the workaround was executed to perform this operation. If `noreturn` or
`fetch_latest_metadata` options were set, they were ignored on storage
side before this patch.

1. tarantool/tarantool#3378
@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/gh-403-names-as-keys branch from 39ca62d to 76a620c Compare December 25, 2023 13:32
@better0fdead better0fdead self-requested a review December 25, 2023 13:36
@DifferentialOrange DifferentialOrange merged commit 56f324d into master Dec 25, 2023
28 checks passed
@DifferentialOrange DifferentialOrange deleted the DifferentialOrange/gh-403-names-as-keys branch December 25, 2023 13:40
DifferentialOrange added a commit that referenced this pull request Dec 25, 2023
DifferentialOrange added a commit that referenced this pull request Dec 25, 2023
Overview

  This release introduces compatibility with vshard 0.1.25 `name_as_key`
  identification mode, as well as several minor fixes and tests
  stabilization.

Added
  * `mode` option for `crud.min` and `crud.max` (#404).

Fixed
  * Compatibility with vshard 0.1.25 `name_as_key` identification mode
    for Tarantool 3.0 (#403).
  * Propagating `noreturn` and `fetch_latest_metadata` options in case
    of intermediate nullable fields update for Tarantool 2.7 and
    older (#404).

Infrastructure
  * Fix flaky update unflatten test case (#404).
  * Do not rely on replication in read test cases (#404).
@DifferentialOrange DifferentialOrange mentioned this pull request Dec 25, 2023
DifferentialOrange added a commit that referenced this pull request Dec 25, 2023
DifferentialOrange added a commit that referenced this pull request Dec 25, 2023
Overview

  This release introduces compatibility with vshard 0.1.25 `name_as_key`
  identification mode, as well as several minor fixes and tests
  stabilization.

Added
  * `mode` option for `crud.min` and `crud.max` (#404).

Fixed
  * Compatibility with vshard 0.1.25 `name_as_key` identification mode
    for Tarantool 3.0 (#403).
  * Propagating `noreturn` and `fetch_latest_metadata` options in case
    of intermediate nullable fields update for Tarantool 2.7 and
    older (#404).

Infrastructure
  * Fix flaky update unflatten test case (#404).
  * Do not rely on replication in read test cases (#404).
DifferentialOrange added a commit that referenced this pull request Dec 26, 2023
Some read cases were missing in #404 test stabilization commit. It
resulted in CI fail [1]. This patch updates missing read cases.

1. https://github.com/tarantool/crud/actions/runs/7322196516/job/19943422319

Follows #404
DifferentialOrange added a commit that referenced this pull request Dec 26, 2023
Some read cases were missing in #404 test stabilization commit. It
resulted in CI fail [1]. This patch updates missing read cases.

1. https://github.com/tarantool/crud/actions/runs/7322196516/job/19943422319

Follows #404
DifferentialOrange added a commit that referenced this pull request Dec 27, 2023
Some read cases were missing in #404 test stabilization commit. It
resulted in CI fail [1]. This patch updates missing read cases.

1. https://github.com/tarantool/crud/actions/runs/7322196516/job/19943422319

Follows #404
DifferentialOrange added a commit that referenced this pull request Dec 28, 2023
Some read cases were missing in #404 test stabilization commit. This
patch updates missing read cases.

1. https://github.com/tarantool/crud/actions/runs/7322196516/job/19943422319

Follows #404
DifferentialOrange added a commit that referenced this pull request Dec 28, 2023
PR #404 has introduced vshard 0.1.25 + Tarantool 3.0 "name as key"
identification mode based on UUIDs extraction. If works fine if
vshard configuration (or Tarantool 3.0 configuration which builds
vshard one) provides UUIDs, but fails if it isn't. Since UUIDs are
optional and won't be provided in most cases, it makes crud fails
to work on most Tarantool 3.0 vshard clusters. This patch fixes
the issue.

Now the code uses name as key, if corresponding mode is enabled, and
uuid otherwise. Patch doesn't cover `select_old` since it runs only
on pre-3.0 Tarantool. Unfortunately, code relies on vshard internals
since now there is no other way [1].

This patch covers new mode support for readview code as well. It likely
was broken before this patch even if UUIDs were provided.

1. tarantool/vshard#460

Follows #404
Closes #407
DifferentialOrange added a commit that referenced this pull request Dec 28, 2023
PR #404 has introduced vshard 0.1.25 + Tarantool 3.0 "name as key"
identification mode based on UUIDs extraction. If works fine if
vshard configuration (or Tarantool 3.0 configuration which builds
vshard one) provides UUIDs, but fails if it isn't. Since UUIDs are
optional and won't be provided in most cases, it makes crud fails
to work on most Tarantool 3.0 vshard clusters. This patch fixes
the issue.

Now the code uses name as key, if corresponding mode is enabled, and
uuid otherwise. Patch doesn't cover `select_old` since it runs only
on pre-3.0 Tarantool. Unfortunately, code relies on vshard internals
since now there is no other way [1].

This patch covers new mode support for readview code as well. It likely
was broken before this patch even if UUIDs were provided.

1. tarantool/vshard#460

Follows #404
Closes #407
DifferentialOrange added a commit that referenced this pull request Dec 29, 2023
PR #404 has introduced vshard 0.1.25 + Tarantool 3.0 "name as key"
identification mode based on UUIDs extraction. If works fine if
vshard configuration (or Tarantool 3.0 configuration which builds
vshard one) provides UUIDs, but fails if it isn't. Since UUIDs are
optional and won't be provided in most cases, it makes crud fails
to work on most Tarantool 3.0 vshard clusters. This patch fixes
the issue.

Now the code uses name as key, if corresponding mode is enabled, and
uuid otherwise. Patch doesn't cover `select_old` since it runs only
on pre-3.0 Tarantool. Unfortunately, code relies on vshard internals
since now there is no other way [1].

This patch covers new mode support for readview code as well. It likely
was broken before this patch even if UUIDs were provided.

1. tarantool/vshard#460

Follows #404
Closes #407
DifferentialOrange added a commit that referenced this pull request Dec 29, 2023
PR #404 has introduced vshard 0.1.25 + Tarantool 3.0 "name as key"
identification mode based on UUIDs extraction. If works fine if
vshard configuration (or Tarantool 3.0 configuration which builds
vshard one) provides UUIDs, but fails if it isn't. Since UUIDs are
optional and won't be provided in most cases, it makes crud fails
to work on most Tarantool 3.0 vshard clusters. This patch fixes
the issue.

Now the code uses name as key, if corresponding mode is enabled, and
uuid otherwise. Patch doesn't cover `select_old` since it runs only
on pre-3.0 Tarantool. Unfortunately, code relies on vshard internals
since now there is no other way [1].

This patch covers new mode support for readview code as well. It likely
was broken before this patch even if UUIDs were provided.

1. tarantool/vshard#460

Follows #404
Part of #407
DifferentialOrange added a commit that referenced this pull request Dec 29, 2023
PR #404 has introduced vshard 0.1.25 + Tarantool 3.0 "name as key"
identification mode based on UUIDs extraction. If works fine if
vshard configuration (or Tarantool 3.0 configuration which builds
vshard one) provides UUIDs, but fails if it isn't. Since UUIDs are
optional and won't be provided in most cases, it makes crud fails
to work on most Tarantool 3.0 vshard clusters. This patch fixes
the issue.

Now the code uses name as key, if corresponding mode is enabled, and
uuid otherwise. Patch doesn't cover `select_old` since it runs only
on pre-3.0 Tarantool. Unfortunately, code relies on vshard internals
since now there is no other way [1].

This patch covers new mode support for readview code as well. It likely
was broken before this patch even if UUIDs were provided.

1. tarantool/vshard#460

Follows #404
Closes #407
DifferentialOrange added a commit that referenced this pull request Jan 9, 2024
PR #404 has introduced vshard 0.1.25 + Tarantool 3.0 "name as key"
identification mode based on UUIDs extraction. If works fine if
vshard configuration (or Tarantool 3.0 configuration which builds
vshard one) provides UUIDs, but fails if it isn't. Since UUIDs are
optional and won't be provided in most cases, it makes crud fails
to work on most Tarantool 3.0 vshard clusters. This patch fixes
the issue.

Now the code uses name as key, if corresponding mode is enabled, and
uuid otherwise. Patch doesn't cover `select_old` since it runs only
on pre-3.0 Tarantool. Unfortunately, code relies on vshard internals
since now there is no other way [1].

This patch covers new mode support for readview code as well. It likely
was broken before this patch even if UUIDs were provided.

1. tarantool/vshard#460

Follows #404
Closes #407
DifferentialOrange added a commit that referenced this pull request Jan 9, 2024
Some read cases were missing in #404 test stabilization commit. This
patch updates missing read cases.

1. https://github.com/tarantool/crud/actions/runs/7322196516/job/19943422319

Follows #404
DifferentialOrange added a commit that referenced this pull request Jan 9, 2024
PR #404 has introduced vshard 0.1.25 + Tarantool 3.0 "name as key"
identification mode based on UUIDs extraction. If works fine if
vshard configuration (or Tarantool 3.0 configuration which builds
vshard one) provides UUIDs, but fails if it isn't. Since UUIDs are
optional and won't be provided in most cases, it makes crud fails
to work on most Tarantool 3.0 vshard clusters. This patch fixes
the issue.

Now the code uses name as key, if corresponding mode is enabled, and
uuid otherwise. Patch doesn't cover `select_old` since it runs only
on pre-3.0 Tarantool. Unfortunately, code relies on vshard internals
since now there is no other way [1].

This patch covers new mode support for readview code as well. It likely
was broken before this patch even if UUIDs were provided.

1. tarantool/vshard#460

Follows #404
Closes #407
DifferentialOrange added a commit that referenced this pull request Jan 10, 2024
Some read cases were missing in #404 test stabilization commit. This
patch updates missing read cases.

1. https://github.com/tarantool/crud/actions/runs/7322196516/job/19943422319

Follows #404
DifferentialOrange added a commit that referenced this pull request Jan 10, 2024
PR #404 has introduced vshard 0.1.25 + Tarantool 3.0 "name as key"
identification mode based on UUIDs extraction. If works fine if
vshard configuration (or Tarantool 3.0 configuration which builds
vshard one) provides UUIDs, but fails if it isn't. Since UUIDs are
optional and won't be provided in most cases, it makes crud fails
to work on most Tarantool 3.0 vshard clusters. This patch fixes
the issue.

Now the code uses name as key, if corresponding mode is enabled, and
uuid otherwise. Patch doesn't cover `select_old` since it runs only
on pre-3.0 Tarantool. Unfortunately, code relies on vshard internals
since now there is no other way [1].

This patch covers new mode support for readview code as well. It likely
was broken before this patch even if UUIDs were provided.

1. tarantool/vshard#460

Follows #404
Closes #407
DifferentialOrange added a commit that referenced this pull request Jan 23, 2024
Some read cases were missing in #404, #406 and #408 test stabilization
commits. This patch updates missing read cases.

1. https://github.com/tarantool/crud/actions/runs/7322196516/job/19943422319

Follows #404
DifferentialOrange added a commit that referenced this pull request Jan 24, 2024
Some read cases were missing in #404, #406 and #408 test stabilization
commits. This patch updates missing read cases.

1. https://github.com/tarantool/crud/actions/runs/7322196516/job/19943422319

Follows #404
DifferentialOrange added a commit that referenced this pull request Mar 22, 2024
Some read cases were missing in #404, #406 and #408 test stabilization
commits. This patch updates missing read cases.

1. https://github.com/tarantool/crud/actions/runs/7322196516/job/19943422319

Follows #404
DifferentialOrange added a commit that referenced this pull request Mar 22, 2024
Some read cases were missing in #404, #406 and #408 test stabilization
commits. This patch updates missing read cases.

1. https://github.com/tarantool/crud/actions/runs/7322196516/job/19943422319

Follows #404
DifferentialOrange added a commit that referenced this pull request Mar 25, 2024
Some read cases were missing in #404, #406 and #408 test stabilization
commits. This patch updates missing read cases.

1. https://github.com/tarantool/crud/actions/runs/7322196516/job/19943422319

Follows #404
Part of #412
Part of #415
DifferentialOrange added a commit that referenced this pull request Mar 27, 2024
Some read cases were missing in #404, #406 and #408 test stabilization
commits. This patch updates missing read cases.

1. https://github.com/tarantool/crud/actions/runs/7322196516/job/19943422319

Follows #404
Part of #412
Part of #415
DifferentialOrange added a commit that referenced this pull request May 20, 2024
Some read cases were missing in #404, #406 and #408 test stabilization
commits. This patch updates missing read cases.

1. https://github.com/tarantool/crud/actions/runs/7322196516/job/19943422319

Follows #404
Part of #412
Part of #415
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.

Support vshard's identification_mode = name_as_key
2 participants