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.

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

tests: base id_allocator_stm_test on raft_fixture #18280

Merged
merged 4 commits into from May 13, 2024
Merged

tests: base id_allocator_stm_test on raft_fixture #18280

merged 4 commits into from May 13, 2024

Conversation

bashtanov
Copy link
Contributor

@bashtanov bashtanov commented May 7, 2024 •
edited

As part of deprecating simple_raft_fixture in favour of raft_fixture, making id_allocator_stm_test to use the latter. For compatibility with raft_fixture::retry_with_leader I had to change id_allocator_stm return type to result. I had to improve raft_fixture::retry_with_leader to accept more data types too. I also created some test helper macros, they are only used occasionally here but will get more use in my next PR(s).

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.1.x
  • v23.3.x
  • v23.2.x

Release Notes

  • none

@CLAassistant
Copy link

CLAassistant commented May 7, 2024 •
edited

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@bashtanov bashtanov force-pushed the base-id_allocator_stm_test-on-raft_fixture branch 2 times, most recently from d7384e8 to 99e6fb3 Compare May 7, 2024 10:58
@bashtanov
Copy link
Contributor Author

/dt

@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented May 7, 2024 •
edited

new failures in https://buildkite.com/redpanda/redpanda/builds/48781#018f5321-3a78-4f3b-bff9-39c2cde069f9:

"rptest.tests.e2e_shadow_indexing_test.EndToEndThrottlingTest.test_throttling.cloud_storage_type=CloudStorageType.ABS"

new failures in https://buildkite.com/redpanda/redpanda/builds/48781#018f5321-3a7b-4cfd-9c75-6d5034936b4e:

"rptest.tests.e2e_shadow_indexing_test.EndToEndThrottlingTest.test_throttling.cloud_storage_type=CloudStorageType.S3"

new failures in https://buildkite.com/redpanda/redpanda/builds/48785#018f53ac-e129-45cd-9289-ca08a4148165:

"rptest.tests.e2e_shadow_indexing_test.EndToEndThrottlingTest.test_throttling.cloud_storage_type=CloudStorageType.ABS"

new failures in https://buildkite.com/redpanda/redpanda/builds/48785#018f53ac-e12b-4b20-abc4-b058a93fd550:

"rptest.tests.e2e_shadow_indexing_test.EndToEndThrottlingTest.test_throttling.cloud_storage_type=CloudStorageType.S3"

new failures in https://buildkite.com/redpanda/redpanda/builds/48785#018f53b4-5ca1-42cb-91e9-a26772408edf:

"rptest.tests.e2e_shadow_indexing_test.EndToEndThrottlingTest.test_throttling.cloud_storage_type=CloudStorageType.S3"

new failures in https://buildkite.com/redpanda/redpanda/builds/48785#018f53b4-5c9e-47e3-a864-30539aba47d1:

"rptest.tests.e2e_shadow_indexing_test.EndToEndThrottlingTest.test_throttling.cloud_storage_type=CloudStorageType.ABS"

new failures in https://buildkite.com/redpanda/redpanda/builds/48790#018f546e-fb22-4cfc-a890-870a1be691d5:

"rptest.tests.e2e_shadow_indexing_test.EndToEndThrottlingTest.test_throttling.cloud_storage_type=CloudStorageType.ABS"

new failures in https://buildkite.com/redpanda/redpanda/builds/48790#018f546e-fb25-44ba-b41f-651f4c271305:

"rptest.tests.e2e_shadow_indexing_test.EndToEndThrottlingTest.test_throttling.cloud_storage_type=CloudStorageType.S3"

new failures in https://buildkite.com/redpanda/redpanda/builds/48790#018f5477-0213-4485-a798-528d0b161800:

"rptest.tests.e2e_shadow_indexing_test.EndToEndThrottlingTest.test_throttling.cloud_storage_type=CloudStorageType.S3"

new failures in https://buildkite.com/redpanda/redpanda/builds/48790#018f5477-0211-4845-962e-a455dc9bb27c:

"rptest.tests.e2e_shadow_indexing_test.EndToEndThrottlingTest.test_throttling.cloud_storage_type=CloudStorageType.ABS"

new failures in https://buildkite.com/redpanda/redpanda/builds/48821#018f580d-990c-4364-b52b-9c1def0728a5:

"rptest.tests.e2e_shadow_indexing_test.EndToEndThrottlingTest.test_throttling.cloud_storage_type=CloudStorageType.ABS"

new failures in https://buildkite.com/redpanda/redpanda/builds/48821#018f580d-990e-4db0-9472-86a3281e650f:

"rptest.tests.e2e_shadow_indexing_test.EndToEndThrottlingTest.test_throttling.cloud_storage_type=CloudStorageType.S3"

new failures in https://buildkite.com/redpanda/redpanda/builds/48821#018f5816-13e7-4169-933f-2b88b423ba97:

"rptest.tests.e2e_shadow_indexing_test.EndToEndThrottlingTest.test_throttling.cloud_storage_type=CloudStorageType.S3"

new failures in https://buildkite.com/redpanda/redpanda/builds/48829#018f58b7-b256-4786-97d0-4f0dc8f6935d:

"rptest.tests.e2e_shadow_indexing_test.EndToEndThrottlingTest.test_throttling.cloud_storage_type=CloudStorageType.ABS"

new failures in https://buildkite.com/redpanda/redpanda/builds/48829#018f58b7-b259-4b8a-9bfa-f63789ae43db:

"rptest.tests.e2e_shadow_indexing_test.EndToEndThrottlingTest.test_throttling.cloud_storage_type=CloudStorageType.S3"

new failures in https://buildkite.com/redpanda/redpanda/builds/48829#018f58c0-8896-49c9-881a-9dfadd6b51d9:

"rptest.tests.e2e_shadow_indexing_test.EndToEndThrottlingTest.test_throttling.cloud_storage_type=CloudStorageType.S3"

new failures in https://buildkite.com/redpanda/redpanda/builds/48829#018f58c0-8893-40b6-a328-f6786a5d361b:

"rptest.tests.e2e_shadow_indexing_test.EndToEndThrottlingTest.test_throttling.cloud_storage_type=CloudStorageType.ABS"

@bashtanov bashtanov force-pushed the base-id_allocator_stm_test-on-raft_fixture branch from 99e6fb3 to d946a43 Compare May 7, 2024 13:56
@bashtanov
Copy link
Contributor Author

/dt

@bashtanov bashtanov marked this pull request as ready for review May 7, 2024 17:56
bharathv
bharathv reviewed May 7, 2024
View reviewed changes
Copy link
Contributor

@bharathv bharathv left a comment

Choose a reason for hiding this comment

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

couple of nits, lgtm otherwise.

src/v/cluster/id_allocator_stm.h Outdated Show resolved Hide resolved
src/v/cluster/tests/id_allocator_stm_test.cc Outdated Show resolved Hide resolved
@bashtanov bashtanov force-pushed the base-id_allocator_stm_test-on-raft_fixture branch from d946a43 to 603b0c6 Compare May 8, 2024 10:23
@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented May 8, 2024 •
edited

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/48821#018f580d-990e-4db0-9472-86a3281e650f

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/48858#018f5ad9-f995-46fd-93ae-dbebbdb385eb

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/48858#018f5ad9-f98f-4bf2-9bab-b1c4406df1c5

@bashtanov bashtanov force-pushed the base-id_allocator_stm_test-on-raft_fixture branch from 603b0c6 to 950e8dc Compare May 8, 2024 13:51
bharathv
bharathv reviewed May 8, 2024
View reviewed changes
Copy link
Contributor

@bharathv bharathv left a comment

Choose a reason for hiding this comment

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

lgtm only pending comment about vassert()

src/v/cluster/tests/id_allocator_stm_test.cc Outdated Show resolved Hide resolved
@bashtanov bashtanov force-pushed the base-id_allocator_stm_test-on-raft_fixture branch 2 times, most recently from e04b9b6 to df78f41 Compare May 8, 2024 23:26
@bashtanov
Copy link
Contributor Author

bashtanov commented May 8, 2024 •
edited

@bharathv I removed all vasserts, thanks for the suggestions

bharathv
bharathv previously approved these changes May 9, 2024
View reviewed changes
@bharathv bharathv requested a review from mmaslankaprv May 9, 2024 15:03
@bashtanov bashtanov force-pushed the base-id_allocator_stm_test-on-raft_fixture branch 2 times, most recently from 808a1fb to 1f23b86 Compare May 10, 2024 12:47
@bashtanov
tests: improve raft_fixture::retry_with_leader error processing
94e99f0 
Support std::error_code, raft::errc, cluster::errc and bool,
both individually and as part of a result<..., ...>
@bashtanov
id_allocator: make it return a result<...> instead of a homemade type
07e714f 
That's mostly for later use in raft_fixture::retry_with_leader
@bashtanov
tests: base id_allocator_stm_test on raft_fixture
3821433 
As part of deprecating simple_raft_fixture in favour of raft_fixture,
making id_allocator_stm_test to use the latter.
@bashtanov bashtanov force-pushed the base-id_allocator_stm_test-on-raft_fixture branch from 1f23b86 to 3821433 Compare May 10, 2024 23:04
@bashtanov
Copy link
Contributor Author

@mmaslankaprv I have reworked the PR to allow use of plain result<...>. I would appreciate if you and/or @bharathv / @ztlpn could re-review. The test failure looks unrelated.

@bashtanov
Copy link
Contributor Author

Test failure is #13275

@mmaslankaprv mmaslankaprv merged commit cf0430c into redpanda-data:dev May 13, 2024
15 of 18 checks passed
dotnwat
dotnwat reviewed May 15, 2024
View reviewed changes
Copy link
Member

@dotnwat dotnwat left a comment

Choose a reason for hiding this comment

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

lgtm

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

@dotnwat dotnwat dotnwat left review comments

@mmaslankaprv mmaslankaprv mmaslankaprv approved these changes

@bharathv bharathv bharathv left review comments

@ztlpn ztlpn Awaiting requested review from ztlpn

Assignees
No one assigned
Labels
area/redpanda
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@bashtanov @CLAassistant @vbotbuildovich @dotnwat @bharathv @mmaslankaprv