From 3a2eb3b6130ae2d1038257aa533c517716ac165f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Tue, 18 Jun 2024 12:38:47 +0200 Subject: [PATCH] Remove file_handle_cache_stats module The stats were not removed from management agent, instead they are hardcoded to zero in the agent itself. --- .../src/rabbit_classic_queue_index_v2.erl | 2 - deps/rabbit/src/rabbit_mnesia.erl | 8 +-- deps/rabbit/src/rabbit_msg_store.erl | 3 - deps/rabbit/src/rabbit_queue_index.erl | 3 - deps/rabbit_common/app.bzl | 3 - deps/rabbit_common/src/file_handle_cache.erl | 15 ++--- .../src/file_handle_cache_stats.erl | 63 ------------------- .../src/rabbit_mgmt_external_stats.erl | 15 ++++- moduleindex.yaml | 1 - 9 files changed, 21 insertions(+), 92 deletions(-) delete mode 100644 deps/rabbit_common/src/file_handle_cache_stats.erl diff --git a/deps/rabbit/src/rabbit_classic_queue_index_v2.erl b/deps/rabbit/src/rabbit_classic_queue_index_v2.erl index cf1e3ec23e64..6781840a7faa 100644 --- a/deps/rabbit/src/rabbit_classic_queue_index_v2.erl +++ b/deps/rabbit/src/rabbit_classic_queue_index_v2.erl @@ -699,7 +699,6 @@ flush_buffer(State0 = #qi { write_buffer = WriteBuffer0, {Fd, FoldState} = get_fd_for_segment(Segment, FoldState1), LocBytes = flush_buffer_consolidate(lists:sort(LocBytes0), 1), ok = file:pwrite(Fd, LocBytes), - file_handle_cache_stats:update(queue_index_write), FoldState end, State0, Writes), %% Update the cache. If we are flushing the entire write buffer, @@ -961,7 +960,6 @@ read_from_disk(SeqIdsToRead0, State0 = #qi{ write_buffer = WriteBuffer }, Acc0) ReadSize = (LastSeqId - FirstSeqId + 1) * ?ENTRY_SIZE, case get_fd(FirstSeqId, State0) of {Fd, OffsetForSeqId, State} -> - file_handle_cache_stats:update(queue_index_read), %% When reading further than the end of a partial file, %% file:pread/3 will return what it could read. case file:pread(Fd, OffsetForSeqId, ReadSize) of diff --git a/deps/rabbit/src/rabbit_mnesia.erl b/deps/rabbit/src/rabbit_mnesia.erl index cd251c35e299..c5ce4843c8d7 100644 --- a/deps/rabbit/src/rabbit_mnesia.erl +++ b/deps/rabbit/src/rabbit_mnesia.erl @@ -817,12 +817,8 @@ execute_mnesia_transaction(TxFun) -> Res = mnesia:sync_transaction(TxFun), DiskLogAfter = mnesia_dumper:get_log_writes(), case DiskLogAfter == DiskLogBefore of - true -> file_handle_cache_stats:update( - mnesia_ram_tx), - Res; - false -> file_handle_cache_stats:update( - mnesia_disk_tx), - {sync, Res} + true -> Res; + false -> {sync, Res} end; true -> mnesia:sync_transaction(TxFun) end diff --git a/deps/rabbit/src/rabbit_msg_store.erl b/deps/rabbit/src/rabbit_msg_store.erl index b5be0acce203..1a360b9edf65 100644 --- a/deps/rabbit/src/rabbit_msg_store.erl +++ b/deps/rabbit/src/rabbit_msg_store.erl @@ -473,7 +473,6 @@ write(MsgRef, MsgId, Msg, CState) -> client_write(MsgRef, MsgId, Msg, noflow, CS read(MsgId, CState = #client_msstate { cur_file_cache_ets = CurFileCacheEts }) -> - file_handle_cache_stats:update(msg_store_read), %% Check the cur file cache case ets:lookup(CurFileCacheEts, MsgId) of [] -> @@ -495,7 +494,6 @@ read_many(_, CState = #client_msstate{ index_module = IndexMod }) when IndexMod =/= rabbit_msg_store_ets_index -> {#{}, CState}; read_many(MsgIds, CState) -> - file_handle_cache_stats:inc(msg_store_read, length(MsgIds)), %% We receive MsgIds in rouhgly the younger->older order so %% we can look for messages in the cache directly. read_many_cache(MsgIds, CState, #{}). @@ -610,7 +608,6 @@ client_write(MsgRef, MsgId, Msg, Flow, CState = #client_msstate { flying_ets = FlyingEts, cur_file_cache_ets = CurFileCacheEts, client_ref = CRef }) -> - file_handle_cache_stats:update(msg_store_write), %% We are guaranteed that the insert will succeed. %% This is true even for queue crashes because CRef will change. true = ets:insert_new(FlyingEts, {{CRef, MsgRef}, ?FLYING_WRITE}), diff --git a/deps/rabbit/src/rabbit_queue_index.erl b/deps/rabbit/src/rabbit_queue_index.erl index 249e870af775..77c47c42df2d 100644 --- a/deps/rabbit/src/rabbit_queue_index.erl +++ b/deps/rabbit/src/rabbit_queue_index.erl @@ -922,8 +922,6 @@ append_journal_to_segment(#segment { journal_entries = JEntries, case array:sparse_size(JEntries) of 0 -> Segment; _ -> - file_handle_cache_stats:update(queue_index_write), - {ok, Hdl} = file_handle_cache:open_with_absolute_path( Path, ?WRITE_MODE, [{write_buffer, infinity}]), @@ -1172,7 +1170,6 @@ load_segment(KeepAcked, #segment { path = Path }) -> case rabbit_file:is_file(Path) of false -> Empty; true -> Size = rabbit_file:file_size(Path), - file_handle_cache_stats:update(queue_index_read), {ok, Hdl} = file_handle_cache:open_with_absolute_path( Path, ?READ_MODE, []), {ok, 0} = file_handle_cache:position(Hdl, bof), diff --git a/deps/rabbit_common/app.bzl b/deps/rabbit_common/app.bzl index e81949b8b189..1c1c77aa0d32 100644 --- a/deps/rabbit_common/app.bzl +++ b/deps/rabbit_common/app.bzl @@ -29,7 +29,6 @@ def all_beam_files(name = "all_beam_files"): "src/delegate.erl", "src/delegate_sup.erl", "src/file_handle_cache.erl", - "src/file_handle_cache_stats.erl", "src/mirrored_supervisor_locks.erl", "src/mnesia_sync.erl", "src/pmon.erl", @@ -125,7 +124,6 @@ def all_test_beam_files(name = "all_test_beam_files"): "src/delegate.erl", "src/delegate_sup.erl", "src/file_handle_cache.erl", - "src/file_handle_cache_stats.erl", "src/mirrored_supervisor_locks.erl", "src/mnesia_sync.erl", "src/pmon.erl", @@ -213,7 +211,6 @@ def all_srcs(name = "all_srcs"): "src/delegate.erl", "src/delegate_sup.erl", "src/file_handle_cache.erl", - "src/file_handle_cache_stats.erl", "src/gen_server2.erl", "src/mirrored_supervisor_locks.erl", "src/mnesia_sync.erl", diff --git a/deps/rabbit_common/src/file_handle_cache.erl b/deps/rabbit_common/src/file_handle_cache.erl index 53eaa80decab..4e5c7901a30c 100644 --- a/deps/rabbit_common/src/file_handle_cache.erl +++ b/deps/rabbit_common/src/file_handle_cache.erl @@ -660,19 +660,16 @@ get_client_state(Pid) -> %%---------------------------------------------------------------------------- prim_file_read(Hdl, Size) -> - file_handle_cache_stats:update( - io_read, Size, fun() -> prim_file:read(Hdl, Size) end). + prim_file:read(Hdl, Size). prim_file_write(Hdl, Bytes) -> - file_handle_cache_stats:update( - io_write, iolist_size(Bytes), fun() -> prim_file:write(Hdl, Bytes) end). + prim_file:write(Hdl, Bytes). prim_file_sync(Hdl) -> - file_handle_cache_stats:update(io_sync, fun() -> prim_file:sync(Hdl) end). + prim_file:sync(Hdl). prim_file_position(Hdl, NewOffset) -> - file_handle_cache_stats:update( - io_seek, fun() -> prim_file:position(Hdl, NewOffset) end). + prim_file:position(Hdl, NewOffset). is_reader(Mode) -> lists:member(read, Mode). @@ -766,8 +763,7 @@ reopen([{Ref, NewOrReopen, Handle = #handle { hdl = closed, RefNewOrReopenHdls] = ToOpen, Tree, RefHdls) -> Mode = case NewOrReopen of new -> Mode0; - reopen -> file_handle_cache_stats:update(io_reopen), - [read | Mode0] + reopen -> [read | Mode0] end, case prim_file:open(Path, Mode) of {ok, Hdl} -> @@ -1103,7 +1099,6 @@ used(#fhc_state{open_count = C1, %%---------------------------------------------------------------------------- init([AlarmSet, AlarmClear]) -> - _ = file_handle_cache_stats:init(), Limit = case application:get_env(file_handles_high_watermark) of {ok, Watermark} when (is_integer(Watermark) andalso Watermark > 0) -> diff --git a/deps/rabbit_common/src/file_handle_cache_stats.erl b/deps/rabbit_common/src/file_handle_cache_stats.erl deleted file mode 100644 index 7b8eb920ad67..000000000000 --- a/deps/rabbit_common/src/file_handle_cache_stats.erl +++ /dev/null @@ -1,63 +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-2024 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. -%% - --module(file_handle_cache_stats). - -%% stats about read / write operations that go through the fhc. - --export([init/0, update/3, update/2, update/1, inc/2, get/0]). - --define(TABLE, ?MODULE). - --define(COUNT, - [io_reopen, mnesia_ram_tx, mnesia_disk_tx, - msg_store_read, msg_store_write, - queue_index_write, queue_index_read]). --define(COUNT_TIME, [io_sync, io_seek]). --define(COUNT_TIME_BYTES, [io_read, io_write]). - --import(rabbit_misc, [safe_ets_update_counter/3, safe_ets_update_counter/4]). - -init() -> - _ = ets:new(?TABLE, [public, named_table, {write_concurrency,true}]), - [ets:insert(?TABLE, {{Op, Counter}, 0}) || Op <- ?COUNT_TIME_BYTES, - Counter <- [count, bytes, time]], - [ets:insert(?TABLE, {{Op, Counter}, 0}) || Op <- ?COUNT_TIME, - Counter <- [count, time]], - [ets:insert(?TABLE, {{Op, Counter}, 0}) || Op <- ?COUNT, - Counter <- [count]]. - -update(Op, Bytes, Thunk) -> - {Time, Res} = timer_tc(Thunk), - _ = safe_ets_update_counter(?TABLE, {Op, count}, 1), - _ = safe_ets_update_counter(?TABLE, {Op, bytes}, Bytes), - _ = safe_ets_update_counter(?TABLE, {Op, time}, Time), - Res. - -update(Op, Thunk) -> - {Time, Res} = timer_tc(Thunk), - _ = safe_ets_update_counter(?TABLE, {Op, count}, 1), - _ = safe_ets_update_counter(?TABLE, {Op, time}, Time), - Res. - -update(Op) -> - _ = safe_ets_update_counter(?TABLE, {Op, count}, 1), - ok. - -inc(Op, Count) -> - _ = safe_ets_update_counter(?TABLE, {Op, count}, Count), - ok. - -get() -> - lists:sort(ets:tab2list(?TABLE)). - -timer_tc(Thunk) -> - T1 = erlang:monotonic_time(), - Res = Thunk(), - T2 = erlang:monotonic_time(), - Diff = erlang:convert_time_unit(T2 - T1, native, micro_seconds), - {Diff, Res}. diff --git a/deps/rabbitmq_management_agent/src/rabbit_mgmt_external_stats.erl b/deps/rabbitmq_management_agent/src/rabbit_mgmt_external_stats.erl index 6b2b6f55be8e..7a78f8bb30f2 100644 --- a/deps/rabbitmq_management_agent/src/rabbit_mgmt_external_stats.erl +++ b/deps/rabbitmq_management_agent/src/rabbit_mgmt_external_stats.erl @@ -414,10 +414,23 @@ update_state(State0) -> FHC = get_fhc_stats(), State0#state{fhc_stats = FHC}. +%% @todo All these stats are zeroes. Remove eventually. get_fhc_stats() -> dict:to_list(dict:merge(fun(_, V1, V2) -> V1 + V2 end, - dict:from_list(file_handle_cache_stats:get()), + dict:from_list(zero_fhc_stats()), dict:from_list(get_ra_io_metrics()))). +zero_fhc_stats() -> + [{{Op, Counter}, 0} || Op <- [io_read, io_write], + Counter <- [count, bytes, time]] + ++ + [{{Op, Counter}, 0} || Op <- [io_sync, io_seek], + Counter <- [count, time]] + ++ + [{{Op, Counter}, 0} || Op <- [io_reopen, mnesia_ram_tx, mnesia_disk_tx, + msg_store_read, msg_store_write, + queue_index_write, queue_index_read], + Counter <- [count]]. + get_ra_io_metrics() -> lists:sort(ets:tab2list(ra_io_metrics)). diff --git a/moduleindex.yaml b/moduleindex.yaml index 9673bcc804c8..5bf2959a9756 100755 --- a/moduleindex.yaml +++ b/moduleindex.yaml @@ -769,7 +769,6 @@ rabbit_common: - delegate - delegate_sup - file_handle_cache -- file_handle_cache_stats - gen_server2 - mirrored_supervisor_locks - mnesia_sync