You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Lukasz Antoniak (Jira)" <ji...@apache.org> on 2024/11/18 05:45:00 UTC
[jira] [Comment Edited] (CASSGO-22) Don't mutate Query/Batch
[ https://issues.apache.org/jira/browse/CASSGO-22?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17898720#comment-17898720 ]
Lukasz Antoniak edited comment on CASSGO-22 at 11/18/24 5:44 AM:
-----------------------------------------------------------------
*Task 1: Move page state handling from Query to Iter*
Backward compatibility: Yes.
Problem statement: Page state is tracked inside the query, causing it impossible to reuse Query object and create two completely separated iterators.
Proposed solution: Move page state tracking under iterator. Page state has to be however left also under Query object, because users may indicate that they would like to execute query starting at given page (aka. reusing page state form Java [driver documentation|https://docs.datastax.com/en/developer/java-driver/4.10/manual/core/paging/index.html#saving-and-reusing-the-paging-state]). Proposed change will require passing Iter next to Query structure in function execution chain starting at fetching next page:
{code:go}
func (n *nextIter) fetch() *Iter {
n.once.Do(func() {
// if the query was specifically run on a connection then re-use that
// connection when fetching the next results
qry := n.iter.qry.(*Query)
if qry.conn != nil {
n.next = qry.conn.executeQuery(qry.Context(), qry, n.iter)
} else {
n.next = qry.session.executeQuery(qry, n.iter)
}
})
return n.next
}
{code}
Iterator needs to be passed until composing query parameters:
{code:go}
func (c *Conn) executeQuery(ctx context.Context, qry *Query, it *Iter) *Iter {
params := queryParams{
consistency: qry.cons,
}
...
if it != nil && it.next != nil && len(it.next.pageState) > 0 {
params.pagingState = it.next.pageState
} else if len(qry.pageState) > 0 {
params.pagingState = qry.pageState
}
if qry.pageSize > 0 {
params.pageSize = qry.pageSize
}
...
}
{code}
*Task 2: Move query metrics under Iter instead of Query and Batch structures*
Backward compatibility: No, users will have to lookup execution attempts and latency from Iter instead of Query or Batch. Custom implementations of RetryPolicy will not be able to cast RetryableQuery to Query or Batch.
{code:go}
// 1.x code
q := session.Query("INSERT INTO test (id, val) VALUES (?, ?)", 1, 1)
if err := query.Exec(); err != nil {
...
}
attempts := q.Attempts()
// 2.0 code
q := session.Query("INSERT INTO test (id, val) VALUES (?, ?)", 1, 1)
iter := q.Iter()
if err := query.Close(); err != nil {
...
}
attempts := iter.Attempts()
{code}
Proposed solution: Moving metrics to Iter impacts the way RetryPolicy is invoked. Query and Batch can no longer implement RetryableQuery interface, because they do not hold execution count (metrics). Instead, we can have Iter implement RetryableQuery. Such change will impact custom RetryPolicy implementations, because users will not be able to cast RetryableQuery to Query or Batch. We have to keep in mind that DowngradingConsistencyRetryPolicy changes consistency level of retried query, which also should NOT affect original, immutable Query object. Therefore, the most straight forward solution could be an Iter to maintain a reference to Query that it executes. At the same time Query needs to be cloned before each retry, so that changing consistency level will not change state of original Query.
{code:go}
type Iter struct {
qry ExecutableQuery
metrics *queryMetrics
...
}
{code}
Iterator is basically a product of single query execution. When it needs to handle final state after several retries, we need to merge mutable part of iterator structure after each retry attempt:
{code:go}
var iter *Iter
for {
...
ni := q.attemptQuery(ctx, qry, it, conn)
ni.merge(iter)
iter = ni
}
{code}
Merge function will combine execution metrics and potentially other properties, so that iterator returned to the user contains complete state.
was (Author: lukasz.antoniak):
*Task 1: Move page state handling from Query to Iter*
Backward compatibility: Yes.
Problem statement: Page state is tracked inside the query, causing it impossible to reuse Query object and create two completely separated iterators.
Proposed solution: Move page state tracking under iterator. Page state has to be however left also under Query object, because users may indicate that they would like to execute query starting at given page (aka. reusing page state form Java [driver documentation|https://docs.datastax.com/en/developer/java-driver/4.10/manual/core/paging/index.html#saving-and-reusing-the-paging-state]). Proposed change will require passing Iter next to Query structure in function execution chain starting at fetching next page:
{code:go}
func (n *nextIter) fetch() *Iter {
n.once.Do(func() {
// if the query was specifically run on a connection then re-use that
// connection when fetching the next results
qry := n.iter.qry.(*Query)
if qry.conn != nil {
n.next = qry.conn.executeQuery(qry.Context(), qry, n.iter)
} else {
n.next = qry.session.executeQuery(qry, n.iter)
}
})
return n.next
}
{code}
Iterator needs to be passed until composing query parameters:
{code:go}
func (c *Conn) executeQuery(ctx context.Context, qry *Query, it *Iter) *Iter {
params := queryParams{
consistency: qry.cons,
}
...
if it != nil && it.next != nil && len(it.next.pageState) > 0 {
params.pagingState = it.next.pageState
} else if len(qry.pageState) > 0 {
params.pagingState = qry.pageState
}
if qry.pageSize > 0 {
params.pageSize = qry.pageSize
}
...
}
{code}
*Task 2: Move query metrics under Iter instead of Query and Batch structures*
Backward compatibility: No, users will have to lookup execution attempts and latency from Iter instead of Query or Batch. Custom implementations of RetryPolicy will not be able to cast RetryableQuery to Query or Batch.
Proposed solution: Moving metrics to Iter impacts the way RetryPolicy is invoked. Query and Batch can no longer implement RetryableQuery interface, because they do not hold execution count (metrics). Instead, we can have Iter implement RetryableQuery. Such change will impact custom RetryPolicy implementations, because users will not be able to cast RetryableQuery to Query or Batch. We have to keep in mind that DowngradingConsistencyRetryPolicy changes consistency level of retried query, which also should NOT affect original, immutable Query object. Therefore, the most straight forward solution could be an Iter to maintain a reference to Query that it executes. At the same time Query needs to be cloned before each retry, so that changing consistency level will not change state of original Query.
{code:go}
type Iter struct {
qry ExecutableQuery
metrics *queryMetrics
...
}
{code}
Iterator is basically a product of single query execution. When it needs to handle final state after several retries, we need to merge mutable part of iterator structure after each retry attempt:
{code:go}
var iter *Iter
for {
...
ni := q.attemptQuery(ctx, qry, it, conn)
ni.merge(iter)
iter = ni
}
{code}
Merge function will combine execution metrics and potentially other properties, so that iterator returned to the user contains complete state.
> Don't mutate Query/Batch
> -------------------------
>
> Key: CASSGO-22
> URL: https://issues.apache.org/jira/browse/CASSGO-22
> Project: Apache Cassandra Go driver
> Issue Type: Improvement
> Components: Core
> Reporter: João Reis
> Assignee: Lukasz Antoniak
> Priority: Normal
> Fix For: 2.0.0
>
>
> https://github.com/apache/cassandra-gocql-driver/issues/1447
> {quote}Currently queries are mutated while being executed, the query metrics and the consistency are both changed. Instead move query metrics to the Iter and remove the need to change the queries consistency for speculation.{quote}
> {quote}Also page state is mutated and the metrics are shared between multiple queries in a single iter (so attempt count is not accurate for queries) - #1508 and #1511 are related.{quote}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org