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

cdc::topology_description: avoid large allocation #15302

Closed
kbr-scylla opened this issue Sep 6, 2023 · 4 comments
Closed

cdc::topology_description: avoid large allocation #15302

kbr-scylla opened this issue Sep 6, 2023 · 4 comments

Comments

@kbr-scylla
Copy link
Contributor

/* Describes a mapping of tokens to CDC streams in a whole token ring.
 *
 * Division of the ring to token ranges is defined in terms of `token_range_end`s
 * in the `_entries` vector. See the comment above `token_range_description` for explanation.
 */
class topology_description {
    std::vector<token_range_description> _entries;
public:
    topology_description(std::vector<token_range_description> entries);
    bool operator==(const topology_description&) const;

    const std::vector<token_range_description>& entries() const&;
    std::vector<token_range_description>&& entries() &&;
};

As observed in https://github.com/scylladb/scylla-enterprise/issues/3387#issuecomment-1706965061, the vector may cause a large allocation -- there could be many token_range_descriptions (it's number_of_nodes * vnodes_per_node).

We need to change this to chunked_vector, for example.

@kbr-scylla kbr-scylla added the n00b label Sep 6, 2023
@mykaul mykaul added this to the 6.0 milestone Sep 7, 2023
@mykaul mykaul added the P2 High Priority label Sep 7, 2023
michaelhly added a commit to michaelhly/scylladb that referenced this issue Sep 15, 2023
Lists can grow very big. Let's use a chunked vector to prevent large contiguous
allocations. Fixes: scylladb#15302.
michaelhly added a commit to michaelhly/scylladb that referenced this issue Sep 15, 2023
Lists can grow very big. Let's use a chunked vector to prevent large contiguous
allocations. Fixes: scylladb#15302.
michaelhly added a commit to michaelhly/scylladb that referenced this issue Sep 18, 2023
Lists can grow very big. Let's use a chunked vector to prevent large contiguous
allocations.
Fixes: scylladb#15302.
@denesb
Copy link
Contributor

denesb commented Dec 18, 2023

Doesn't apply cleanly. Since this was done by an external contributor, @kbr-scylla can you please take care of backporting this?

kbr-scylla pushed a commit that referenced this issue Dec 19, 2023
Lists can grow very big. Let's use a chunked vector to prevent large contiguous
allocations.
Fixes: #15302.

Closes #15428

(cherry picked from commit 62a8a31)
@kbr-scylla
Copy link
Contributor Author

Backported to 5.2

@kbr-scylla
Copy link
Contributor Author

I backported to open source, but forgot about the ancient enterprise LTS versions. Need to backport there too

@kbr-scylla
Copy link
Contributor Author

kbr-scylla commented Apr 19, 2024

Queued backports to enterprise branches:
2022.2: scylladb/scylla-enterprise@7f605a2012df6fab4565de8f4dbbb5d1f619fb68
2022.1: scylladb/scylla-enterprise@b62863ad7bada8a1e6734e127e876e79bab16d89
2021.1: scylladb/scylla-enterprise@e40d4abdf7a30453c126cb73debdb2cbd03dbaf6

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

No branches or pull requests

5 participants