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

Support of sending queries to the specific node #1793

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

worryg0d
Copy link

@worryg0d worryg0d commented Aug 5, 2024

Overview

This PR provides a mechanism that allows users to specify on which node the query will be executed. It is now a typical use case, but it makes sense with virtual tables.
For example, when we want to retrieve metrics for a specific node, we have to send queries to the associated system view of this node.

Implementation Overview

A new method SetHost() for the Query allows users to specify the host (node) on which we have to send the query. It is implemented by adding a new private interface:

type hostGetter interface {
	getHost() *HostInfo
}

When the queryExecutor.executeQuery is called then it checks if the provided ExecutableQuery implements the hostGetter interface as well. We need this interface to not change the API ExecutableQuery. Also, the ExecutableQuery is implemented by Query and Batch as well, so it is good to avoid adding redundant methods for Batch as well.

So, if provided ExecutableQuery implements hostGetter, then it type-asserts it and calls the underlying method to get the host. If the host is not nil, it wraps the host into hostIter func which just returns the specified host.

This PR also exposes to users session.GetHosts() method which calls the underlying session.hostSource.GetHosts() method.

Usage Example

Here is an example of retrieving metrics from system_view.cql_metrics for each node of the cluster:

cluster := NewCluster("127.0.0.1")
session, err := cluster.CreateSession()
if err != nil {
	panic(err)
}
defer session.Close()

hosts, err := session.GetHosts()
if err != nil {
	panic(err)
}

type cqlMetric struct {
	name  string
	value float64
}

// hostId to []cqlMetric
cqlMetrics := map[string][]cqlMetric{}

for _, host := range hosts {
	iter := session.Query("SELECT * FROM system_views.cql_metrics").
		SetHost(host).
		Iter()

	metrics := make([]cqlMetric, 0, iter.NumRows())
	metric := cqlMetric{}
	for iter.Scan(&metric.name, &metric.value) {
		metrics = append(metrics, metric)
	}
	cqlMetrics[host.HostID()] = metrics
}

@martin-sucha
Copy link
Contributor

Should the API allow for other routing preferences? For example only a single data center/rack? Or be extensible to allow targeting a specific shard in the scylladb/gocql fork?

Should the API allow to route to a different host during retry, i.e. allow passing a HostSelectionPolicy or a similar interface/function?

@worryg0d
Copy link
Author

worryg0d commented Aug 6, 2024

Should the API allow for other routing preferences? For example only a single data center/rack? Or be extensible to allow targeting a specific shard in the scylladb/gocql fork?

Should the API allow to route to a different host during retry, i.e. allow passing a HostSelectionPolicy or a similar interface/function?

HostInfo provides API to retrieve information like dc or rack for the host, so users may filter retrieved hosts manually before binding them to the query.

In my opinion, re-routing queries bound to the specific hosts to others is not a good idea even during retries, at least as default behavior. Sending queries to a specific host is not a typical use case and if we try to do it, we assume that the specified host is reachable. Or, at least, we expect that the query was attempted to be sent to the specified host.

The idea of this API being extensible sounds good, but I am not sure about implementation. May you have some advice or ideas on how to properly implement this?

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.

4 participants