[jira] [Commented] (CASSANDRA-17148) Adapt track_warnings to Guardrails

["Andres de la Peña (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CASSANDRA-17148?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17477856#comment-17477856 ] 

Andres de la Peña commented on CASSANDRA-17148:
-----------------------------------------------

As an initial step, the following patch moves the configuration and JMX methods for track_warnings to guardrails:
||PR||CI||
|[trunk|https://github.com/apache/cassandra/pull/1406]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/1245/workflows/2f2d6f91-3f19-4f2f-bd72-84b13adf11cf] [j11|https://app.circleci.com/pipelines/github/adelapena/cassandra/1245/workflows/f19f1c10-3795-48c9-899f-9957ecb563e6]|

That way, the configuration would look like:
{code:java}
guardrails:
  enabled: false
  ...
  coordinator_read_size:
    warn_threshold_in_kb: -1
    abort_threshold_in_kb: -1
  local_read_size:
    warn_threshold_in_kb: -1
    abort_threshold_in_kb: -1
  row_index_size:
    warn_threshold_in_kb: -1
    abort_threshold_in_kb: -1
{code}
The patch only changes configuration and JMX methods, which is the minimum that we would need before making a release so we don't need to deprecate any current properties in the near future. The changes don't go beyond this, so nothing in track_warnings is extending the {{Guardrail}} class and, conversely, nothing in {{Guardrail}} and its extending classes is changed.

The next step would be getting to a tighter integration between both types of threshold. I guess we should either extending the capabilities of the current {{o.a.c.db.guardrails.Threshold}} class, or adding a new subclass of {{Guardrail}} specifically for track_warnings. Some of the things to consider are:
 * track warnings throw dedicated, specific exceptions ({{{}ReadSizeAbortException{}}}), while all guardrails throw the same type of exception.
 * track_warnings update specific metrics, while guardrails don't track any metrics. Metric recording can be easily added to guardrails, maybe with the listeners that we discarded during CASSANDRA-17147.
 * IIUC {{coordinator_read_size}} accumulates multiple checks before issuing a warning, so we don't emit multiple warnings. The class {{Threshold.Counter}} that we discarded during CASSANDRA-17147 was meant to do this, we can bring it back to life.
 * IIUC track warnings are applied to all queries, whereas guardrails are not applied to super user nor internal queries. Those queries are ignored checking the query's {{{}ClientState{}}}, which doesn't seem easy to get for {{local_read_size}} and {{{}row_index_size{}}}.

I wonder if we should break this into two tickets, so we first set the common the configuration that is visible to users and then we try to get a better integration, or do it all in the same patch. [~dcapwell] what do you think?
 

> Adapt track_warnings to Guardrails
> ----------------------------------
>
>                 Key: CASSANDRA-17148
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-17148
>             Project: Cassandra
>          Issue Type: New Feature
>          Components: Feature/Guardrails
>            Reporter: Andres de la Peña
>            Assignee: Andres de la Peña
>            Priority: Normal
>
> The purpose of this ticket is adapting the warnings added by CASSANDRA-16850 to use the new Guardrails framework. This will involve extending the framework to be able to propagate warnings and failures triggered on replicas, taking advantage of the machinery added in that ticket.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org