Skip to content

Conversation

@hmmr
Copy link

@hmmr hmmr commented Oct 28, 2025

This implements martinsumner#478.

When called here, it returns a map containing reportable items described in the issue above.

(dev2@127.0.0.1)12> riak_kv_vnode:vnode_status(riak_core_vnode_manager:all_index_pid(riak_kv_vnode)).
[{1347321821914426127719021955160323408745312813056,
  [{backend_status,riak_kv_leveled_backend,
                   #{ledger_cache_size => 4,
                     journal_last_compaction_result => {0,0.0},
                     get_sample_count => 0,get_body_time => 0,
                     head_sample_count => 0,head_rsp_time => 0,
                     put_sample_count => 0,put_prep_time => 0,put_ink_time => 0,
                     put_mem_time => 0}},
   {vnodeid,<<"}ÍyرYdå">>},
   {counter,140003},
   {counter_lease,150000},
   {counter_lease_size,10000},
   {counter_leasing,false}]},
...

Not all items can be available at the time of calling, in which case the backend_status will not include the corresponding keys.

In riak_control, the results will show as follows:
Screenshot_2025-10-29_01-11-07
Screenshot_2025-10-29_01-11-33

@hmmr
Copy link
Author

hmmr commented Oct 28, 2025

Marking as WIP pending work on the last item ("Mean level for recent fetches").

@hmmr hmmr changed the title WIP leveled_bookie:status/1 leveled_bookie:status/1 Oct 29, 2025
@martinsumner martinsumner moved this to Todo in OpenRiak 3.4.1 Jan 7, 2026
@martinsumner martinsumner moved this from Todo to Ready For Review in OpenRiak 3.4.1 Jan 7, 2026
@martinsumner martinsumner self-requested a review January 7, 2026 13:25
@martinsumner
Copy link

Can we add a test to the ct tests, or perhaps just extend one of the tests in basic_SUITE.

It would be good to verify that the results returned are as expected. Some may just need to be range checked, but you can get the PIDs of the Inker/Penciller via leveled_bookie:book_returnactors/1 to absolutely verify things like the penciller cache size.

Obviously fails the coverage check at the moment as book_status/1 is never called:

|------------------------|------------|
  |                module  |  coverage  |
  |------------------------|------------|
  |        leveled_bookie  |       99%  |
  |           leveled_cdb  |      100%  |
  |         leveled_codec  |      100%  |
  |        leveled_ebloom  |      100%  |
  |          leveled_eval  |      100%  |
  |        leveled_filter  |      100%  |
  |          leveled_head  |      100%  |
  |        leveled_iclerk  |      100%  |
  |     leveled_imanifest  |      100%  |
  |         leveled_inker  |      100%  |
  |           leveled_log  |      100%  |
  |       leveled_monitor  |       97%  |
  |        leveled_pclerk  |      100%  |
  |     leveled_penciller  |      100%  |
  |     leveled_pmanifest  |      100%  |
  |          leveled_pmem  |      100%  |
  |        leveled_runner  |      100%  |
  |         leveled_setop  |      100%  |
  |           leveled_sst  |      100%  |
  |      leveled_sstblock  |      100%  |
  |        leveled_tictac  |      100%  |
  |          leveled_tree  |      100%  |
  |          leveled_util  |      100%  |
  |------------------------|------------|
  |                 total  |       99%  |
  |------------------------|------------|

There are tests in basic_SUITE that prompt journal compaction, so you cna follow the pattern to check results are expected after a compaction.

@hmmr hmmr changed the title leveled_bookie:status/1 WIP leveled_bookie:status/1 Jan 15, 2026
@martinsumner
Copy link

The PUT will call leveled_monitor:maybe_time/2 - so sometimes it will be timed, and sometimes not. So I'm not sure that the test will pass reliably.

I do think it should try a number of operations, so that we can check all the values change to the right order of magnitude. There are a lot of load functions, or you can add a fetch and validation of the book status into one of the other tests.

Also, what happened to reporting the journal compaction scores and results?


-spec book_status(pid()) -> proplists:proplist().
%% @doc
%% Return a proplist conteaining the following items:

Choose a reason for hiding this comment

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

containing

book_headstatus(Pid) ->
gen_server:call(Pid, head_status, infinity).

-spec book_status(pid()) -> proplists:proplist().

Choose a reason for hiding this comment

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

A proplist or a map?

put_mem_time => PT#bookie_put_timings.mem_time,
fetch_count_by_level => FCL
}.

Choose a reason for hiding this comment

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

If there has been no compaction, is there no report on compaction? Would it not be better to consistently sure all keys exist, but have them set to a default value when they haven't occurred.

code_change(_OldVsn, State, _Extra) ->
{ok, State}.

enriched_bookie_status(#state{
Copy link

@martinsumner martinsumner Jan 20, 2026

Choose a reason for hiding this comment

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

dialyzer spec required.

By default, in leveled, we try to avoid passing the LoopState to functions outside of the standard handle_call/caast/info functions. I would probably do this within the handle_call rather than have it as a separate function - as it is just a function based on knowledge of #state, and there's not separate testing of the function. [in this case there would be no need for a dialyzer spec]

@martinsumner
Copy link

One other thing to note, which came up in a thread about stats - is the riak admin vnode-status command.

I think it would be worth looking at the formatting of the output of this CLI command as part of this. There's an open issue from the basho days: basho/riak_kv#343

As we have the OTP json module available, perhaps we could fix this by just pretty-printing json, so that we don't end up coupling some presentation logic in the CLI to the format of the status output.

@hmmr hmmr force-pushed the tiot/openriak-3.4/leveled_bookie_status branch from def17f2 to 11e30b9 Compare January 23, 2026 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready For Review

Development

Successfully merging this pull request may close these issues.

2 participants