Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: one leftover Arc::ptr_eq #3573

Merged
merged 2 commits into from
Feb 9, 2023
Merged

fix: one leftover Arc::ptr_eq #3573

merged 2 commits into from
Feb 9, 2023

Conversation

koivunej
Copy link
Member

@koivunej koivunej commented Feb 9, 2023

@knizhnik noticed that one instance of Arc::<dyn PersistentLayer>::ptr_eq was missed in #3558.

Now all ptr_eq which remain are in comments.

@koivunej koivunej requested review from a team as code owners February 9, 2023 09:29
@koivunej koivunej requested review from shanyp and removed request for a team February 9, 2023 09:29
@koivunej
Copy link
Member Author

koivunej commented Feb 9, 2023

Remaining are:

pageserver/src/tenant/layer_map.rs
301:            // this assertion is related to use of Arc::ptr_eq in Self::compare_arced_layers,
734:    /// Similar to `Arc::ptr_eq`, but only compares the object pointers, not vtables.
744:        // `Arc::ptr_eq` as of writing (at least up to 1.67) uses a comparison where both the

pageserver/src/tenant/layer_map/historic_layer_coverage.rs
436:    /// against some expectation. This allows to use `Arc::ptr_eq` or similar which would be

As I've checked a few times now. I never do remember search all in the first place, as I thought I had already dedupped the callsites when introducing compare_arced_layers.

@koivunej koivunej enabled auto-merge (squash) February 9, 2023 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants