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

queue stats logging refinements #1875

Open
wants to merge 7 commits into
base: develop-2.9
Choose a base branch
from

Conversation

hmmr
Copy link
Contributor

@hmmr hmmr commented Nov 1, 2023

This PR has a few refinements to logging for 2.9.x users:

  • Exposes replrtq_logfrequency in riak.conf. This tunable was already consulted but wasn't present in riak_kv.schema, resulting in application:get_value on that key always returning the default value of 30 sec.
  • Suppresses logging of queue stats when queue sizes are 0.
  • Adds a metadata item, queue_name, to the periodic queue stats logging, to enable selective logging into separate lager sinks.

@hmmr
Copy link
Contributor Author

hmmr commented Nov 1, 2023

An example configuration with filtering:

{lager,
 [
  {extra_sinks,
   [
    {object_lager_event,
     [{handlers,
       [{lager_file_backend,
         [{file, "./log/object.log"},
          {level, info},
          {formatter_config, [date, " ", time," [",severity,"] ",message, "\n"]}
         ]
        }]
      },
      {async_threshold, 500},
      {async_threshold_window, 50}]
    }
   ]
  },
  {traces, [
            {{lager_file_backend, "./log/tta_noise.log"}, [{queue_name, '=', q1_ttaaefs}], info}
           ]}
 ]
}

@martinsumner
Copy link
Contributor

Some comments, but no strong opinion against merging this as-is.

I think there are issues if you upgrade riak, but have config items that are not included in the next riak release. So if we introduce this schema element in 2.9, anyone upgrading to 3.0 will need to remove it from riak.conf. I haven't verified this, working from memory.

It probably makes sense to make the name more general (i.e. queue_manager_log_frequency), and then add the same schema item with the same effect in develop-3.0/develop. Avoids this issue, and also the reaper/eraser processes aren't directly related to replrtq, so this would be a truer meaning.

I'm happy for empty queues to be debug by default. I do know that there is a customer that may prefer to still receive these logs. Should there also be a queue_manager_suppress_empty_log config flag which is off by default), so that this can be controlled - especially as you're taking something away that is currently used to feed production monitoring charts in that environment.

In general, looks fine to me.

@hmmr
Copy link
Contributor Author

hmmr commented Nov 4, 2023

I have renamed the log frequency tunable as you suggested, and reverted separate log levels for zero/non-zero stats, in favor of it being controlled by another tunable, queue_manager_suppress_empty_log.

I'm going to create similar PRs for 3.0 and 3.2 branches.

Indeed, adding a new parameter to riak.conf will cause a slight hiccup in the upgrade procedure, but once we add these tunables to the upcoming releases of 3.0 and 3.2 lines, we should be good.

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.

None yet

2 participants