joao-r-reis commented on code in PR #1790:
URL:
https://github.com/apache/cassandra-gocql-driver/pull/1790#discussion_r1838274399
##########
conn.go:
##########
@@ -1689,36 +1689,37 @@ func (c *Conn) querySystemLocal(ctx context.Context)
*Iter {
func (c *Conn) awaitSchemaAgreement(ctx context.Context) (err error) {
const localSchemas = "SELECT schema_version FROM system.local WHERE
key='local'"
- var versions map[string]struct{}
+ versions := make(map[string]struct{})
var schemaVersion string
endDeadline := time.Now().Add(c.session.cfg.MaxWaitSchemaAgreement)
for time.Now().Before(endDeadline) {
- iter := c.querySystemPeers(ctx, c.host.version)
+ iter := &Iter{}
+ if !c.session.cfg.DisableHostLookup {
+ iter = c.querySystemPeers(ctx, c.host.version)
- versions = make(map[string]struct{})
-
- rows, err := iter.SliceMap()
- if err != nil {
- goto cont
- }
-
- for _, row := range rows {
- host, err := c.session.hostInfoFromMap(row,
&HostInfo{connectAddress: c.host.ConnectAddress(), port: c.session.cfg.Port})
+ rows, err := iter.SliceMap()
if err != nil {
goto cont
}
- if !isValidPeer(host) || host.schemaVersion == "" {
- c.logger.Printf("invalid peer or peer with
empty schema_version: peer=%q", host)
- continue
- }
- versions[host.schemaVersion] = struct{}{}
- }
+ for _, row := range rows {
+ host, err := c.session.hostInfoFromMap(row,
&HostInfo{connectAddress: c.host.ConnectAddress(), port: c.session.cfg.Port})
+ if err != nil {
+ goto cont
+ }
+ if !isValidPeer(host) || host.schemaVersion ==
"" {
+ c.logger.Printf("invalid peer or peer
with empty schema_version: peer=%q", host)
+ continue
+ }
- if err = iter.Close(); err != nil {
- goto cont
+ versions[host.schemaVersion] = struct{}{}
+ }
+
+ if err = iter.Close(); err != nil {
+ goto cont
+ }
Review Comment:
I think it's fine if we query system.peers for the purpose of retrieving the
schema version of each host as long as we don't update gocql's ring/hosts. If a
user wants to use a static set of contact points and avoid using hosts from
system tables there's no reason why they would have to give up the
functionality of awaiting for schema agreement.
##########
cluster.go:
##########
@@ -189,12 +189,12 @@ type ClusterConfig struct {
// set to 10.0.0.1 which is what will be used to connect to.
IgnorePeerAddr bool
- // If DisableInitialHostLookup then the driver will not attempt to get
host info
+ // If DisableHostLookup then the driver will not attempt to get host
info
// from the system.peers table, this will mean that the driver will
connect to
// hosts supplied and will not attempt to lookup the hosts information,
this will
// mean that data_centre, rack and token information will not be
available and as
// such host filtering and token aware query routing will not be
available.
- DisableInitialHostLookup bool
+ DisableHostLookup bool
Review Comment:
I don't think this is worth breaking applications. I agree that removing the
`Initial` word from the name does make sense but it's really not that big of a
deal.
##########
host_source.go:
##########
@@ -635,6 +635,11 @@ func (r *ringDescriber) getClusterPeerInfo(localHost
*HostInfo) ([]*HostInfo, er
return nil, errNoControl
}
+ // Check if host lookup is disabled
+ if r.session.cfg.DisableHostLookup {
+ return []*HostInfo{}, nil
+ }
Review Comment:
Instead of adding this, I think we should make the `ringRefresher` do a NOOP
when this flag is set. `getClusterPeerInfo` should only be about retriving the
peers info from the server so it should be up to the caller whether this should
be done or not.
If we make the `func refreshRing(r *ringDescriber) error` function do
nothing when the flag is set then it should solve all of the issues related to
the driver not respecting the flag.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
