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

Use chunked_vector in append_entries_request #24774

Merged
merged 3 commits into from
Jan 22, 2025

Conversation

mmaslankaprv
Copy link
Member

@mmaslankaprv mmaslankaprv commented Jan 10, 2025

model::record_batch_reader was not the best abstraction to represent
the batches in append_entries_request. Number of batches contained in
the request is known upfront and they are always materialized in memory.
Therefore it is more convenient and easier to store batches as a plain
chunked_vector instead of the reader.

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.3.x
  • v24.2.x
  • v24.1.x

Release Notes

  • none

@mmaslankaprv mmaslankaprv force-pushed the aer-chunked-vector branch 2 times, most recently from 0a6b045 to f09f518 Compare January 13, 2025 10:10
@mmaslankaprv mmaslankaprv changed the title Aer chunked vector Use chunked_vector in append_entries_request Jan 13, 2025
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.

deja vu did we just review a similar pr?

@mmaslankaprv
Copy link
Member Author

kind of similar, this one is a follow up, getting rid of the reader abstraction from RPC request

@mmaslankaprv mmaslankaprv marked this pull request as ready for review January 17, 2025 07:09
@mmaslankaprv mmaslankaprv force-pushed the aer-chunked-vector branch 5 times, most recently from b87f750 to 6bb544f Compare January 20, 2025 13:01
@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Jan 20, 2025

CI test results

test results on build#60953
test_id test_kind job_url test_status passed
gtest_raft_rpunit.gtest_raft_rpunit unit https://buildkite.com/redpanda/redpanda/builds/60953#019483cf-5edb-44c4-b103-4d61503275f7 FLAKY 1/2
rptest.tests.data_migrations_api_test.DataMigrationsApiTest.test_conflicting_names ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-173a-4b08-912a-c4ea07cbc523 FLAKY 1/2
rptest.tests.data_migrations_api_test.DataMigrationsApiTest.test_creating_when_cluster_misconfigured2 ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-173a-4b08-912a-c4ea07cbc523 FLAKY 1/2
rptest.tests.data_migrations_api_test.DataMigrationsApiTest.test_migrated_topic_data_integrity.transfer_leadership=True.params=.cancellation.None.use_alias.False ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-1738-4738-9ab7-ab595829935e FLAKY 1/3
rptest.tests.e2e_iam_role_test.ShortLivedCredentialsTests.test_short_lived_credentials ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-173a-4b08-912a-c4ea07cbc523 FLAKY 1/2
rptest.tests.e2e_shadow_indexing_test.EndToEndHydrationTimeoutTest.test_hydration_completes_on_timeout ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-1738-4738-9ab7-ab595829935e FLAKY 1/2
rptest.tests.e2e_shadow_indexing_test.EndToEndShadowIndexingTest.test_reset_from_cloud.cloud_storage_type=CloudStorageType.ABS ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-1738-4738-9ab7-ab595829935e FLAKY 1/5
rptest.tests.e2e_shadow_indexing_test.EndToEndShadowIndexingTest.test_write.cloud_storage_type=CloudStorageType.ABS ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-1738-4738-9ab7-ab595829935e FLAKY 1/2
rptest.tests.nodes_decommissioning_test.NodesDecommissioningTest.test_decommissioning_cancel_ongoing_movements ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-1738-4738-9ab7-ab595829935e FLAKY 1/2
rptest.tests.partition_balancer_test.PartitionBalancerTest.test_partition_autobalancer_sanction.disable_license=False ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-173a-4b08-912a-c4ea07cbc523 FLAKY 1/2
rptest.tests.partition_balancer_test.PartitionBalancerTest.test_partition_autobalancer_sanction.disable_license=True ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-1738-4738-9ab7-ab595829935e FLAKY 1/2
rptest.tests.partition_force_reconfiguration_test.PartitionForceReconfigurationTest.test_basic_reconfiguration.acks=1.restart=True.controller_snapshots=True ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-173a-4b08-912a-c4ea07cbc523 FLAKY 1/2
rptest.tests.partition_move_interruption_test.PartitionMoveInterruption.test_cancelling_partition_move_node_down ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-173a-4b08-912a-c4ea07cbc523 FLAKY 1/2
rptest.tests.partition_move_interruption_test.PartitionMoveInterruption.test_cancelling_partition_move_x_core.replication_factor=1.unclean_abort=False.recovery=no_recovery.compacted=False ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-1738-4738-9ab7-ab595829935e FLAKY 1/2
rptest.tests.partition_movement_test.PartitionMovementTest.test_overlapping_changes.num_to_upgrade=2 ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-173a-4b08-912a-c4ea07cbc523 FLAKY 1/2
rptest.tests.scaling_up_test.ScalingUpTest.test_adding_node_with_unavailable_node ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-173a-4b08-912a-c4ea07cbc523 FLAKY 1/10
rptest.tests.scaling_up_test.ScalingUpTest.test_on_demand_rebalancing.partition_count=20 ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-173a-4b08-912a-c4ea07cbc523 FLAKY 1/2
rptest.tests.scaling_up_test.ScalingUpTest.test_topic_hot_spots ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-1738-4738-9ab7-ab595829935e FLAKY 1/16
rptest.tests.shadow_indexing_compacted_topic_test.ShadowIndexingCompactedTopicTest.test_upload.cloud_storage_type=CloudStorageType.ABS ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-1738-4738-9ab7-ab595829935e FLAKY 1/11
rptest.tests.shard_placement_test.ShardPlacementTest.test_node_join.disable_license=True ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-1738-4738-9ab7-ab595829935e FLAKY 1/5
rptest.tests.topic_creation_test.TopicRecreateTest.test_topic_recreation_while_producing.workload=ACKS_1.cleanup_policy=delete ducktape https://buildkite.com/redpanda/redpanda/builds/60953#01948414-173a-4b08-912a-c4ea07cbc523 FLAKY 1/2
test results on build#60965
test_id test_kind job_url test_status passed
rptest.tests.controller_log_limiting_test.ControllerLogLimitPartitionBalancerTests.test_partition_balancer_with_limits ducktape https://buildkite.com/redpanda/redpanda/builds/60965#0194853e-9825-4641-9134-890279770f34 FLAKY 1/13
rptest.tests.data_migrations_api_test.DataMigrationsApiTest.test_migrated_topic_data_integrity.transfer_leadership=True.params=.cancellation.dir.in.stage.preparing.use_alias.False ducktape https://buildkite.com/redpanda/redpanda/builds/60965#0194853e-9823-432c-9fb6-7109029b692b FLAKY 1/2
rptest.tests.data_transforms_test.DataTransformsTest.test_consume_off_end.offset=None ducktape https://buildkite.com/redpanda/redpanda/builds/60965#0194853e-9825-4641-9134-890279770f34 FLAKY 1/2
rptest.tests.e2e_iam_role_test.STSRoleFetchTests.test_write ducktape https://buildkite.com/redpanda/redpanda/builds/60965#0194853e-9825-4641-9134-890279770f34 FLAKY 1/2
rptest.tests.e2e_shadow_indexing_test.EndToEndShadowIndexingTest.test_reset_from_cloud.cloud_storage_type=CloudStorageType.ABS ducktape https://buildkite.com/redpanda/redpanda/builds/60965#0194853e-9823-432c-9fb6-7109029b692b FLAKY 1/2
rptest.tests.full_node_recovery_test.FullNodeRecoveryTest.test_node_recovery.recovery_type=full ducktape https://buildkite.com/redpanda/redpanda/builds/60965#0194853e-9825-4641-9134-890279770f34 FLAKY 1/2
rptest.tests.idempotency_test.IdempotencySnapshotDelivery.test_recovery_after_snapshot_is_delivered ducktape https://buildkite.com/redpanda/redpanda/builds/60965#0194853e-9825-4641-9134-890279770f34 FLAKY 1/2
rptest.tests.partition_force_reconfiguration_test.PartitionForceReconfigurationTest.test_basic_reconfiguration.acks=-1.restart=True.controller_snapshots=False ducktape https://buildkite.com/redpanda/redpanda/builds/60965#0194853e-9825-4641-9134-890279770f34 FLAKY 1/3
rptest.tests.partition_move_interruption_test.PartitionMoveInterruption.test_cancelling_partition_move.replication_factor=1.unclean_abort=True.recovery=no_recovery.compacted=True ducktape https://buildkite.com/redpanda/redpanda/builds/60965#0194853e-9823-432c-9fb6-7109029b692b FLAKY 1/2
rptest.tests.raft_periodic_flush_test.PeriodicFlushWithRelaxedConsistencyTest.test_changing_periodic_flush_threshold ducktape https://buildkite.com/redpanda/redpanda/builds/60965#0194853e-9825-4641-9134-890279770f34 FLAKY 1/8
rptest.tests.scaling_up_test.ScalingUpTest.test_adding_multiple_nodes_to_the_cluster.partition_count=20 ducktape https://buildkite.com/redpanda/redpanda/builds/60965#0194853e-9825-4641-9134-890279770f34 FLAKY 1/5
rptest.tests.scaling_up_test.ScalingUpTest.test_topic_hot_spots ducktape https://buildkite.com/redpanda/redpanda/builds/60965#0194853e-9823-432c-9fb6-7109029b692b FLAKY 1/10
rptest.tests.shadow_indexing_compacted_topic_test.ShadowIndexingCompactedTopicTest.test_upload.cloud_storage_type=CloudStorageType.ABS ducktape https://buildkite.com/redpanda/redpanda/builds/60965#0194853e-9823-432c-9fb6-7109029b692b FLAKY 1/2
rptest.tests.shadow_indexing_compacted_topic_test.TSWithAlreadyCompactedTopic.test_initial_upload ducktape https://buildkite.com/redpanda/redpanda/builds/60965#0194853e-9825-4641-9134-890279770f34 FAIL 0/20
test results on build#60987
test_id test_kind job_url test_status passed
rptest.tests.datalake.datalake_e2e_test.DatalakeE2ETests.test_topic_lifecycle.cloud_storage_type=CloudStorageType.S3.filesystem_catalog_mode=True ducktape https://buildkite.com/redpanda/redpanda/builds/60987#019488e9-59a5-40ed-87fb-7ee03ba4f877 FLAKY 1/2
rptest.transactions.producers_api_test.ProducersAdminAPITest.test_producers_state_api_during_load ducktape https://buildkite.com/redpanda/redpanda/builds/60987#019488e9-59a5-4052-bde0-5d364528bc03 FLAKY 1/2
test results on build#61037
test_id test_kind job_url test_status passed
rptest.tests.cloud_storage_chunk_read_path_test.CloudStorageChunkReadTest.test_read_when_segment_size_smaller_than_chunk_size ducktape https://buildkite.com/redpanda/redpanda/builds/61037#01948d9b-c123-4963-a751-bbc9898b394b FLAKY 1/2
rptest.tests.cluster_metadata_upload_test.ClusterMetadataUploadTest.test_uploads_after_restart.cloud_storage_type=CloudStorageType.ABS ducktape https://buildkite.com/redpanda/redpanda/builds/61037#01948d9b-c123-4963-a751-bbc9898b394b FLAKY 1/2

@dotnwat
Copy link
Member

dotnwat commented Jan 20, 2025

linter problem

@vbotbuildovich
Copy link
Collaborator

Retry command for Build#60965

please wait until all jobs are finished before running the slash command

/ci-repeat 1
tests/rptest/tests/shadow_indexing_compacted_topic_test.py::TSWithAlreadyCompactedTopic.test_initial_upload

src/v/raft/tests/raft_fixture.cc Outdated Show resolved Hide resolved
src/v/raft/types.h Outdated Show resolved Hide resolved
src/v/raft/consensus.cc Outdated Show resolved Hide resolved
In order to match the real life scenario changed raft fixture to use
serde wrapper for append entries request as wrapper is used in normal
operations.

Signed-off-by: Michał Maślanka <[email protected]>
`model::record_batch_reader` was not the best abstraction to represent
the batches in `append_entries_request`. Number of batches contained in
the request is known upfront and they are always materialized in memory.
Therefore it is more convenient and easier to store batches as a plain
`chunked_vector` instead of the reader.

Signed-off-by: Michał Maślanka <[email protected]>
@mmaslankaprv mmaslankaprv merged commit 39840ad into redpanda-data:dev Jan 22, 2025
17 checks passed
@mmaslankaprv mmaslankaprv deleted the aer-chunked-vector branch January 23, 2025 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants