[ 
https://issues.apache.org/jira/browse/CASSANDRA-21700?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Michael Semb Wever updated CASSANDRA-21700:
-------------------------------------------
    Description: 
{{IndexHints.fromCQLNames}} rejects a count above the maximum:

{code:java}
if (included != null && included.size() > maxIncludedOrExcludedIndexCount())
    throw new InvalidRequestException(TOO_MANY_INDEXES_ERROR + included.size());
{code}

so a {{SELECT}} may name exactly {{maxIncludedOrExcludedIndexCount()}} included 
or excluded indexes. The serializer then asserted a count strictly below the 
same maximum:

{code:java}
assert n < maxIncludedOrExcludedIndexCount() : TOO_MANY_INDEXES_ERROR + n;
{code}

The two disagree at the boundary. A query that names the maximum passes 
validation on the coordinator and then throws an {{AssertionError}} when the 
read command is serialized for a replica, and the daemon runs with assertions 
enabled. An operator meets this by setting 
{{secondary_indexes_per_table_fail_threshold}} to N and naming N indexes, or by 
naming 128 indexes when that guardrail is unset.

The patch relaxes the assertion to {{<=}}, and makes 
{{maxIncludedOrExcludedIndexCount()}} package-private so the test can read the 
same limit. The count is written with {{writeVInt32}}, so the boundary carries 
no on-the-wire risk.

Patch: 
[mck/CASSANDRA-21700/trunk|https://github.com/thelastpickle/cassandra/tree/mck/CASSANDRA-21700/trunk]
Provenance: 
[d29e4a67f7|https://github.com/datastax/cassandra/commit/d29e4a67f76d2c812f2a3a283910876797ff2f9c]
 by [~brandon.williams]. That commit makes the same one-character change, and 
also ignores an {{UnknownIndexException}} during deserialization so a hint for 
an index that has not yet propagated is dropped rather than failing the read. 
This patch leaves that part out, because silently dropping a hint changes what 
the query does and needs its own discussion. That commit carries no test; this 
patch adds one.

  was:
{{IndexHints.fromCQLNames}} rejects a count above the maximum:

{code:java}
if (included != null && included.size() > maxIncludedOrExcludedIndexCount())
    throw new InvalidRequestException(TOO_MANY_INDEXES_ERROR + included.size());
{code}

so a {{SELECT}} may name exactly {{maxIncludedOrExcludedIndexCount()}} included 
or excluded indexes. The serializer then asserted a count strictly below the 
same maximum:

{code:java}
assert n < maxIncludedOrExcludedIndexCount() : TOO_MANY_INDEXES_ERROR + n;
{code}

The two disagree at the boundary. A query that names the maximum passes 
validation on the coordinator and then throws an {{AssertionError}} when the 
read command is serialized for a replica, and the daemon runs with assertions 
enabled. An operator meets this by setting 
{{secondary_indexes_per_table_fail_threshold}} to N and naming N indexes, or by 
naming 128 indexes when that guardrail is unset.

The patch relaxes the assertion to {{<=}}, and makes 
{{maxIncludedOrExcludedIndexCount()}} package-private so the test can read the 
same limit. The count is written with {{writeVInt32}}, so the boundary carries 
no on-the-wire risk.

Patch: 
[mck/upstream/index-hints-limit-off-by-one/trunk|https://github.com/thelastpickle/cassandra/tree/mck/upstream/index-hints-limit-off-by-one/trunk]
Provenance: 
[d29e4a67f7|https://github.com/datastax/cassandra/commit/d29e4a67f76d2c812f2a3a283910876797ff2f9c]
 by [~brandon.williams]. That commit makes the same one-character change, and 
also ignores an {{UnknownIndexException}} during deserialization so a hint for 
an index that has not yet propagated is dropped rather than failing the read. 
This patch leaves that part out, because silently dropping a hint changes what 
the query does and needs its own discussion. That commit carries no test; this 
patch adds one.


> Index hints serialization rejects a count that the validation accepts
> ---------------------------------------------------------------------
>
>                 Key: CASSANDRA-21700
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-21700
>             Project: Apache Cassandra
>          Issue Type: Bug
>          Components: Feature/2i Index
>            Reporter: Michael Semb Wever
>            Priority: Normal
>             Fix For: 6.0.x, 7.x
>
>
> {{IndexHints.fromCQLNames}} rejects a count above the maximum:
> {code:java}
> if (included != null && included.size() > maxIncludedOrExcludedIndexCount())
>     throw new InvalidRequestException(TOO_MANY_INDEXES_ERROR + 
> included.size());
> {code}
> so a {{SELECT}} may name exactly {{maxIncludedOrExcludedIndexCount()}} 
> included or excluded indexes. The serializer then asserted a count strictly 
> below the same maximum:
> {code:java}
> assert n < maxIncludedOrExcludedIndexCount() : TOO_MANY_INDEXES_ERROR + n;
> {code}
> The two disagree at the boundary. A query that names the maximum passes 
> validation on the coordinator and then throws an {{AssertionError}} when the 
> read command is serialized for a replica, and the daemon runs with assertions 
> enabled. An operator meets this by setting 
> {{secondary_indexes_per_table_fail_threshold}} to N and naming N indexes, or 
> by naming 128 indexes when that guardrail is unset.
> The patch relaxes the assertion to {{<=}}, and makes 
> {{maxIncludedOrExcludedIndexCount()}} package-private so the test can read 
> the same limit. The count is written with {{writeVInt32}}, so the boundary 
> carries no on-the-wire risk.
> Patch: 
> [mck/CASSANDRA-21700/trunk|https://github.com/thelastpickle/cassandra/tree/mck/CASSANDRA-21700/trunk]
> Provenance: 
> [d29e4a67f7|https://github.com/datastax/cassandra/commit/d29e4a67f76d2c812f2a3a283910876797ff2f9c]
>  by [~brandon.williams]. That commit makes the same one-character change, and 
> also ignores an {{UnknownIndexException}} during deserialization so a hint 
> for an index that has not yet propagated is dropped rather than failing the 
> read. This patch leaves that part out, because silently dropping a hint 
> changes what the query does and needs its own discussion. That commit carries 
> no test; this patch adds one.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to