zeroshade commented on code in PR #1806:
URL: https://github.com/apache/iceberg-go/pull/1806#discussion_r3807678294
##########
table/metadata.go:
##########
@@ -1964,10 +2062,14 @@ func (c *commonMetadata) Snapshots() []Snapshot {
}
func (c *commonMetadata) SnapshotByID(id int64) *Snapshot {
- for i := range c.SnapshotList {
- if c.SnapshotList[i].SnapshotID == id {
- return cloneSnapshotPtr(&c.SnapshotList[i])
- }
+ if snapshotIndexNeedsRebuild(c.snapshotIndex, c.SnapshotList) {
+ c.snapshotIndex = buildSnapshotIndex(c.SnapshotList)
+ }
+
+ i, ok, index := snapshotIndexPosition(c.snapshotIndex, c.SnapshotList,
id)
+ c.snapshotIndex = index
Review Comment:
**Major:** This assignment makes every steady-state `SnapshotByID` call
write to published table metadata, even when `index` is the unchanged existing
pointer. Metadata is shared by tables and scans, so parallel calls through
`SnapshotByID`, `SnapshotByName`, or `CurrentSnapshot` now race. A focused
eight-goroutine test under `go test -race` reported read/write and write/write
races at lines 2065, 2069, and 2070. Please keep the normal lookup path
read-only—by completing index initialization before publication or using
concurrency-safe initialization/repair—and add a concurrent lookup regression
test.
--
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]
