zeroshade commented on code in PR #1806:
URL: https://github.com/apache/iceberg-go/pull/1806#discussion_r3832549325
##########
table/metadata.go:
##########
@@ -1211,10 +1304,13 @@ func (b *MetadataBuilder) GetSortOrderByID(id int)
(*SortOrder, error) {
}
func (b *MetadataBuilder) SnapshotByID(id int64) (*Snapshot, error) {
- for _, s := range b.snapshotList {
- if s.SnapshotID == id {
- return &s, nil
- }
+ b.ensureSnapshotIndex()
+ i, ok, index := snapshotIndexPosition(b.snapshotIndex, b.snapshotList,
id)
+ b.snapshotIndex = index
Review Comment:
This unconditional assignment makes concurrent read-only calls to the
exported `MetadataBuilder.SnapshotByID` race, even when `index` is the
unchanged existing pointer. A focused eight-goroutine probe under `go test
-race` reports read/write and write/write races between `ensureSnapshotIndex`
and this line; the base implementation's linear lookup was read-only. Please
keep this lookup path read-only—mirroring `commonMetadata.SnapshotByID` where
appropriate—and add a concurrent builder-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]
