Skip to content
This repository was archived by the owner on Jul 21, 2025. It is now read-only.

Conversation

@Kulezi
Copy link
Contributor

@Kulezi Kulezi commented Sep 9, 2022

Currently transport package atomically replaces topology and connections with new ones. This is done using atomic.Value that hold topology and conn pointers, this is not type-safe and requires type assertions every time the driver needs the values hidden in atomic.Value.

Go 1.19 introduces a generic atomic.Pointer[T], that is a more suitable replacement, adding type-safety and removing the type assertions,

This PR replaces mentioned atomic.Values with the new generic atomic pointer type.

Fixes #269

@Kulezi Kulezi force-pushed the pp/go-1.19-patch branch 3 times, most recently from 5fa174f to bd3757b Compare September 12, 2022 09:47
@Kulezi Kulezi changed the title [WIP] Pp/go 1.19 patch transport: use go 1.19 atomic.Pointer instead of atomic.Value Sep 12, 2022
@Kulezi Kulezi force-pushed the pp/go-1.19-patch branch 4 times, most recently from 9235bba to 15058fa Compare September 12, 2022 10:57
return nil
}

func (c *Cluster) Topology() *topology {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Topology() method still necessary? It is an exported method returning non-exported type, so I wonder if we could just remove it in favor of using c.topology.Load() directly.

Copy link
Contributor Author

@Kulezi Kulezi Sep 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Topology() is used in session to get all nodes to prepare a query on them. Nodes are the only field of topology that is currently exported and used outside of transport, so I suppose Topology() could be removed and replaced with just func (*Cluster) Nodes().

The other thing is that topology is passed inside QueryInfo to host selection policies, but same as all fields of QueryInfo, it is unexported. It likely should be accessible outside of transport, opened #287 about it, it may influence if topology will be exported.

The point of Topology() and setTopology() cluster methods was to give
some sort of type safety as topology was an atomic.Value that can hold
any type. As topology is now an atomic.Pointer they are no longer needed.

Session sometimes needs to perform a query on all nodes of
the cluster, to allow that cluster now has an exported Nodes() method.
@Kulezi Kulezi marked this pull request as ready for review September 14, 2022 09:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

transport: replace atomic.Value with go 1.19 atomic.Pointer where suitable

3 participants