Skip to content

libnetwork/networkdb: switch to go-immutable-radix v2#48157

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:libnetwork_immutable_radix_v2
Jul 31, 2024
Merged

libnetwork/networkdb: switch to go-immutable-radix v2#48157
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:libnetwork_immutable_radix_v2

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah added status/2-code-review area/networking Networking kind/refactor PR's that refactor, or clean-up code labels Jul 11, 2024
@thaJeztah thaJeztah added this to the 28.0.0 milestone Jul 11, 2024
@thaJeztah thaJeztah self-assigned this Jul 11, 2024
@thaJeztah thaJeztah marked this pull request as ready for review July 11, 2024 16:31
Comment thread libnetwork/networkdb/networkdb.go Outdated
Comment on lines +347 to +349
if v == nil {
return nil, nil
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this check needed with v2 but not with v1?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good question; maybe it was needed as well; there's a nil-check two lines up, and my IDE was flagging it as "could be nil"; honestly not sure it there's such a chance

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is at least one other place which assumes that (*NetworkDB).getEntry() error = nil implies that the returned *entry is not nil.

e, err := nDB.getEntry(tEvent.TableName, tEvent.NetworkID, tEvent.Key)
if err == nil {
// We have the latest state. Ignore the event
// since it is stale.
if e.ltime >= tEvent.LTime {
nDB.Unlock()
return false
}
} else if tEvent.Type == TableEventTypeDelete && !isBulkSync {

I am pretty sure that it is an invariant. An incomplete audit of the NetworkDB code suggests that to be the case.

@thaJeztah thaJeztah force-pushed the libnetwork_immutable_radix_v2 branch from 4469c66 to 2847c4b Compare July 26, 2024 21:12
@thaJeztah
Copy link
Copy Markdown
Member Author

@corhere updated; I removed the nil check, and added a comment there to reference the discussion here.

@thaJeztah thaJeztah merged commit 376a699 into moby:master Jul 31, 2024
@thaJeztah thaJeztah deleted the libnetwork_immutable_radix_v2 branch July 31, 2024 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/networking Networking kind/refactor PR's that refactor, or clean-up code status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants