Remove msg_store_io_batch_size and msg_store_credit_disc_bound checks

msg_store_io_batch_size is no longer used

msg_store_credit_disc_bound appears to be used in the code, but I don't
see any impact of that value on the performance. It should be properly
investigated and either removed completely or fixed, because there's
hardly any point in warning about the values configured
(plus, this settings is hopefully almost never used anyway)
This commit is contained in:
Michal Kuratczyk 2025-01-14 17:35:16 +01:00
parent 954b861db7
commit 14171fb035
No known key found for this signature in database
7 changed files with 4 additions and 164 deletions

View File

@ -95,7 +95,6 @@ _APP_ENV = """[
%% writing to the queue index. See the setting
%% queue_index_embed_msgs_below above.
{msg_store_credit_disc_bound, {4000, 800}},
{msg_store_io_batch_size, 4096},
%% see rabbitmq-server#143,
%% rabbitmq-server#949, rabbitmq-server#1098
{credit_flow_default_credit, {400, 200}},
@ -532,14 +531,6 @@ rabbitmq_integration_suite(
],
)
rabbitmq_suite(
name = "msg_store_SUITE",
size = "small",
deps = [
"//deps/rabbit_common:erlang_app",
],
)
rabbitmq_integration_suite(
name = "peer_discovery_classic_config_SUITE",
size = "large",

View File

@ -78,7 +78,6 @@ define PROJECT_ENV
%% writing to the queue index. See the setting
%% queue_index_embed_msgs_below above.
{msg_store_credit_disc_bound, {4000, 800}},
{msg_store_io_batch_size, 4096},
%% see rabbitmq-server#143,
%% rabbitmq-server#949, rabbitmq-server#1098
{credit_flow_default_credit, {400, 200}},
@ -192,7 +191,6 @@ SLOW_CT_SUITES := backing_queue \
health_check \
many_node_ha \
metrics \
msg_store \
partitions \
per_user_connection_tracking \
per_vhost_connection_limit \
@ -272,7 +270,7 @@ PARALLEL_CT_SET_2_D = queue_length_limits queue_parallel quorum_queue_member_rec
PARALLEL_CT_SET_3_A = definition_import per_user_connection_channel_limit_partitions per_vhost_connection_limit_partitions policy priority_queue_recovery rabbit_fifo_prop rabbit_fifo_v0 rabbit_stream_sac_coordinator unit_credit_flow unit_queue_consumers unit_queue_location unit_quorum_queue
PARALLEL_CT_SET_3_B = cluster_upgrade list_consumers_sanity_check list_queues_online_and_offline logging lqueue maintenance_mode rabbit_fifo_q
PARALLEL_CT_SET_3_C = cli_forget_cluster_node feature_flags_v2 mc_unit message_containers_deaths_v2 message_size_limit metadata_store_migration
PARALLEL_CT_SET_3_D = metadata_store_phase1 metrics mirrored_supervisor msg_store peer_discovery_classic_config proxy_protocol runtime_parameters unit_stats_and_metrics unit_supervisor2 unit_vm_memory_monitor
PARALLEL_CT_SET_3_D = metadata_store_phase1 metrics mirrored_supervisor peer_discovery_classic_config proxy_protocol runtime_parameters unit_stats_and_metrics unit_supervisor2 unit_vm_memory_monitor
PARALLEL_CT_SET_4_A = clustering_events rabbit_local_random_exchange rabbit_message_interceptor rabbitmq_4_0_deprecations unit_pg_local unit_plugin_directories unit_plugin_versioning unit_policy_validators unit_priority_queue
PARALLEL_CT_SET_4_B = per_user_connection_tracking per_vhost_connection_limit rabbit_fifo_dlx_integration rabbit_fifo_int

9
deps/rabbit/app.bzl vendored
View File

@ -1073,15 +1073,6 @@ def test_suite_beam_files(name = "test_suite_beam_files"):
beam = ["ebin/mirrored_supervisor.beam"],
erlc_opts = "//:test_erlc_opts",
)
erlang_bytecode(
name = "msg_store_SUITE_beam_files",
testonly = True,
srcs = ["test/msg_store_SUITE.erl"],
outs = ["test/msg_store_SUITE.beam"],
app_name = "rabbit",
erlc_opts = "//:test_erlc_opts",
deps = ["//deps/rabbit_common:erlang_app"],
)
erlang_bytecode(
name = "peer_discovery_classic_config_SUITE_beam_files",
testonly = True,

View File

@ -61,7 +61,6 @@
, metadata_store_phase1_SUITE
, metrics_SUITE
, mirrored_supervisor_SUITE
, msg_store_SUITE
, peer_discovery_classic_config_SUITE
]}.

View File

@ -42,9 +42,6 @@
pg_local_amqp_session/0,
pg_local_amqp_connection/0]).
%% for tests
-export([validate_msg_store_io_batch_size_and_credit_disc_bound/2]).
-rabbit_boot_step({pre_boot, [{description, "rabbit boot start"}]}).
-rabbit_boot_step({codec_correctness_check,
@ -969,7 +966,6 @@ start(normal, []) ->
print_banner(),
log_banner(),
warn_if_kernel_config_dubious(),
warn_if_disc_io_options_dubious(),
?LOG_DEBUG(""),
?LOG_DEBUG("== Plugins (prelaunch phase) =="),
@ -1443,87 +1439,6 @@ warn_if_kernel_config_dubious() ->
true -> ok
end.
warn_if_disc_io_options_dubious() ->
%% if these values are not set, it doesn't matter since
%% rabbit_variable_queue will pick up the values defined in the
%% IO_BATCH_SIZE and CREDIT_DISC_BOUND constants.
CreditDiscBound = rabbit_misc:get_env(rabbit, msg_store_credit_disc_bound,
undefined),
IoBatchSize = rabbit_misc:get_env(rabbit, msg_store_io_batch_size,
undefined),
case catch validate_msg_store_io_batch_size_and_credit_disc_bound(
CreditDiscBound, IoBatchSize) of
ok -> ok;
{error, {Reason, Vars}} ->
?LOG_WARNING(Reason, Vars,
#{domain => ?RMQLOG_DOMAIN_GLOBAL})
end.
validate_msg_store_io_batch_size_and_credit_disc_bound(CreditDiscBound,
IoBatchSize) ->
case IoBatchSize of
undefined ->
ok;
IoBatchSize when is_integer(IoBatchSize) ->
if IoBatchSize < ?IO_BATCH_SIZE ->
throw({error,
{"io_batch_size of ~b lower than recommended value ~b, "
"paging performance may worsen",
[IoBatchSize, ?IO_BATCH_SIZE]}});
true ->
ok
end;
IoBatchSize ->
throw({error,
{"io_batch_size should be an integer, but ~b given",
[IoBatchSize]}})
end,
%% CreditDiscBound = {InitialCredit, MoreCreditAfter}
{RIC, RMCA} = ?CREDIT_DISC_BOUND,
case CreditDiscBound of
undefined ->
ok;
{IC, MCA} when is_integer(IC), is_integer(MCA) ->
if IC < RIC; MCA < RMCA ->
throw({error,
{"msg_store_credit_disc_bound {~b, ~b} lower than"
"recommended value {~b, ~b},"
" paging performance may worsen",
[IC, MCA, RIC, RMCA]}});
true ->
ok
end;
{IC, MCA} ->
throw({error,
{"both msg_store_credit_disc_bound values should be integers, but ~tp given",
[{IC, MCA}]}});
CreditDiscBound ->
throw({error,
{"invalid msg_store_credit_disc_bound value given: ~tp",
[CreditDiscBound]}})
end,
case {CreditDiscBound, IoBatchSize} of
{undefined, undefined} ->
ok;
{_CDB, undefined} ->
ok;
{undefined, _IBS} ->
ok;
{{InitialCredit, _MCA}, IoBatchSize} ->
if IoBatchSize < InitialCredit ->
throw(
{error,
{"msg_store_io_batch_size ~b should be bigger than the initial "
"credit value from msg_store_credit_disc_bound ~b,"
" paging performance may worsen",
[IoBatchSize, InitialCredit]}});
true ->
ok
end
end.
-spec product_name() -> string().
product_name() ->

View File

@ -210,7 +210,7 @@
disk_read_count,
disk_write_count,
io_batch_size,
io_batch_size, %% Unused.
%% default queue or lazy queue
mode, %% Unused.
@ -334,7 +334,7 @@
disk_read_count :: non_neg_integer(),
disk_write_count :: non_neg_integer(),
io_batch_size :: pos_integer(),
io_batch_size :: 0,
mode :: 'default' | 'lazy',
version :: 2,
unconfirmed_simple :: sets:set()}.
@ -1195,8 +1195,6 @@ init(IsDurable, IndexState, StoreState, DeltaCount, DeltaBytes, Terms,
end_seq_id = NextSeqId })
end,
Now = erlang:monotonic_time(),
IoBatchSize = rabbit_misc:get_env(rabbit, msg_store_io_batch_size,
?IO_BATCH_SIZE),
{ok, IndexMaxSize} = application:get_env(
rabbit, queue_index_embed_msgs_below),
@ -1242,7 +1240,7 @@ init(IsDurable, IndexState, StoreState, DeltaCount, DeltaBytes, Terms,
disk_read_count = 0,
disk_write_count = 0,
io_batch_size = IoBatchSize,
io_batch_size = 0,
mode = default,
virtual_host = VHost},

View File

@ -1,52 +0,0 @@
%% This Source Code Form is subject to the terms of the Mozilla Public
%% License, v. 2.0. If a copy of the MPL was not distributed with this
%% file, You can obtain one at https://mozilla.org/MPL/2.0/.
%%
%% Copyright (c) 2007-2025 Broadcom. All Rights Reserved. The term Broadcom refers to Broadcom Inc. and/or its subsidiaries. All rights reserved.
%%
-module(msg_store_SUITE).
-include_lib("rabbit_common/include/rabbit.hrl").
-compile(export_all).
-define(T(Fun, Args), (catch apply(rabbit, Fun, Args))).
all() ->
[
parameter_validation
].
parameter_validation(_Config) ->
%% make sure it works with default values
ok = ?T(validate_msg_store_io_batch_size_and_credit_disc_bound,
[?CREDIT_DISC_BOUND, ?IO_BATCH_SIZE]),
%% IO_BATCH_SIZE must be greater than CREDIT_DISC_BOUND initial credit
ok = ?T(validate_msg_store_io_batch_size_and_credit_disc_bound,
[{4000, 800}, 5000]),
{error, _} = ?T(validate_msg_store_io_batch_size_and_credit_disc_bound,
[{4000, 800}, 1500]),
%% All values must be integers
{error, _} = ?T(validate_msg_store_io_batch_size_and_credit_disc_bound,
[{2000, 500}, "1500"]),
{error, _} = ?T(validate_msg_store_io_batch_size_and_credit_disc_bound,
[{"2000", 500}, abc]),
{error, _} = ?T(validate_msg_store_io_batch_size_and_credit_disc_bound,
[{2000, "500"}, 2048]),
%% CREDIT_DISC_BOUND must be a tuple
{error, _} = ?T(validate_msg_store_io_batch_size_and_credit_disc_bound,
[[2000, 500], 1500]),
{error, _} = ?T(validate_msg_store_io_batch_size_and_credit_disc_bound,
[2000, 1500]),
%% config values can't be smaller than default values
{error, _} = ?T(validate_msg_store_io_batch_size_and_credit_disc_bound,
[{1999, 500}, 2048]),
{error, _} = ?T(validate_msg_store_io_batch_size_and_credit_disc_bound,
[{2000, 499}, 2048]),
{error, _} = ?T(validate_msg_store_io_batch_size_and_credit_disc_bound,
[{2000, 500}, 2047]).