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

config: set schema_commitlog_segment_size_in_mb to 128 #14704

Merged

Conversation

patjed41
Copy link
Contributor

@patjed41 patjed41 commented Jul 14, 2023

Fixes #14668

In #14668, we have decided to introduce a new scylla.yaml variable for the schema commitlog segment size and set it to 128MB. The reason is that segment size puts a limit on the mutation size that can be written at once, and some schema mutation writes are much larger than average, as shown in #13864. This schema_commitlog_segment_size_in_mb variable variable is now added to scylla.yaml and db/config.

Additionally, we do not derive the commitlog sync period for schema commitlog anymore because schema commitlog runs in batch mode, so it doesn't need this parameter. It has also been discussed in #14668.

@patjed41 patjed41 requested review from piodul and kostja July 14, 2023 14:45
@scylladb-promoter
Copy link
Contributor

@avikivity
Copy link
Member

btw, there's a related issue: what's the schema commitlog's hard limit? It should be set to 2*segment size, to avoid the schema commitlog wasting space.

@@ -469,6 +469,9 @@ db::config::config(std::shared_ptr<db::extensions> exts)
, commitlog_segment_size_in_mb(this, "commitlog_segment_size_in_mb", value_status::Used, 64,
"Sets the size of the individual commitlog file segments. A commitlog segment may be archived, deleted, or recycled after all its data has been flushed to SSTables. This amount of data can potentially include commitlog segments from every table in the system. The default size is usually suitable for most commitlog archiving, but if you want a finer granularity, 8 or 16 MB is reasonable. See Commit log archive configuration.\n"
"Related information: Commit log archive configuration")
, schema_commitlog_segment_size_in_mb(this, "schema_commitlog_segment_size_in_mb", value_status::Used, 128,
"Sets the size of the individual schema commitlog file segments. The default size is larger than the default size of the data commitlog because the segment size puts a limit on the mutation size that can be written at once, and some schema mutation writes are much larger than average.\n"
"Related information: Commit log archive configuration")
Copy link
Member

Choose a reason for hiding this comment

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

Please make it LiveUpdate (and verify it responds to changes).

Copy link
Contributor

Choose a reason for hiding this comment

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

The commitlog does not support changing the segment size dynamically. It will be a non-trivial change which needs to be thoroughly tested. This sounds out of scope for this PR.

@piodul
Copy link
Contributor

piodul commented Jul 17, 2023

btw, there's a related issue: what's the schema commitlog's hard limit? It should be set to 2*segment size, to avoid the schema commitlog wasting space.

The size limit is... hardcoded to be 10 terabytes:

c.commitlog_total_space_in_mb = 10 << 20;

cc: @gusev-p @tgrabiec was that intentional?

It does sound like a good idea to lower the limit, but do we actually want a hard limit? We don't enforce a hard limit right now:

c.allow_going_over_size_limit = true; // for lower latency

and, AFAIK, there are some issues with the hard limit and we don't enforce it by default for the regular commitlog: #9625

@patjed41 patjed41 force-pushed the schema-commitlog-segment-size branch from c2f1c99 to f925b18 Compare July 17, 2023 12:29
@patjed41 patjed41 changed the title config: add schema_commitlog_segment_size_in_mb variable config: set schema_commitlog_segment_size_in_mb to 128 Jul 17, 2023
@tgrabiec
Copy link
Contributor

cc: @gusev-p @tgrabiec was that intentional?

No, it should be 10 mb, like the memory limit for the system kesypace region.

@scylladb-promoter
Copy link
Contributor

@kbr-scylla
Copy link
Contributor

Fixes #14668, #13864

How have you verified that this fixes #13864?

@patjed41 patjed41 force-pushed the schema-commitlog-segment-size branch from f925b18 to 8e7f571 Compare July 19, 2023 11:55
@patjed41
Copy link
Contributor Author

patjed41 commented Jul 19, 2023

v2: moved parts of the first commit that should belong to the second commit (to the second commit)

@patjed41 patjed41 force-pushed the schema-commitlog-segment-size branch from 8e7f571 to df16852 Compare July 19, 2023 11:59
In scylladb#14668, we have decided to introduce a new scylla.yaml variable
for the schema commitlog segment size. The segment size puts a limit
on the mutation size that can be written at once, and some schema
mutation writes are much larger than average, as shown in scylladb#13864.
Therefore, increasing the schema commitlog segment size is sometimes
necessary.
We increase the default schema commitlog segment size so that the
large mutations do not fail. We have agreed that 128 MB is sufficient.
We don't want to apply the value of the commitlog_sync_period_in_ms
variable to schema commitlog. Schema commitlog runs in batch mode,
so it doesn't need this parameter.
@patjed41 patjed41 force-pushed the schema-commitlog-segment-size branch from df16852 to ee1c240 Compare July 19, 2023 12:18
@patjed41
Copy link
Contributor Author

patjed41 commented Jul 19, 2023

Fixes #14668, #13864

How have you verified that this fixes #13864?

Not really, since I'm unable to reproduce this issue. @margdoc is working on it. After creating a reproducer, verifying that this PR is indeed a fix should be simple.

I've removed #13864 from the description. This issue will be closed after verifying that this PR fixes it.

@scylladb-promoter
Copy link
Contributor

@patjed41 patjed41 requested a review from kbr-scylla July 21, 2023 12:46
@DoronArazii
Copy link

Ping review please, it's a blocker for us.

@kbr-scylla
Copy link
Contributor

The size limit is... hardcoded to be 10 terabytes:

@piodul can you please create an issue about this?

@piodul
Copy link
Contributor

piodul commented Jul 24, 2023

The size limit is... hardcoded to be 10 terabytes:

@piodul can you please create an issue about this?

#14802

@scylladb-promoter scylladb-promoter merged commit e6099c4 into scylladb:master Jul 24, 2023
3 checks passed
kbr-scylla added a commit that referenced this pull request Aug 2, 2023
…Patryk Jędrzejczak

Fixes #14668

In #14668, we have decided to introduce a new `scylla.yaml` variable for the schema commitlog segment size and set it to 128MB. The reason is that segment size puts a limit on the mutation size that can be written at once, and some schema mutation writes are much larger than average, as shown in #13864. This `schema_commitlog_segment_size_in_mb variable` variable is now added to `scylla.yaml` and `db/config`.

Additionally,  we do not derive the commitlog sync period for schema commitlog anymore because schema commitlog runs in batch mode, so it doesn't need this parameter. It has also been discussed in #14668.

Closes #14704

* github.com:scylladb/scylladb:
  replica: do not derive the commitlog sync period for schema commitlog
  config: set schema_commitlog_segment_size_in_mb to 128
  config: add schema_commitlog_segment_size_in_mb variable

(cherry picked from commit e6099c4)
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.

Increase schema commitlog default segment size
7 participants