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

[Get By Key] get by key with limits #180

Conversation

PranavVadrevu
Copy link

No description provided.

studzien and others added 10 commits December 10, 2021 23:32
This pull request includes a dirty version of the get_by_key function.
It also modifies Tracker.list to use the standard presence list mechanism,
and it introduces dirty_list as a standalone function.
This was supposed to be added in #5.
@PranavVadrevu PranavVadrevu deleted the get_by_key/get_by_key_with_limits branch December 11, 2023 14:11
@PranavVadrevu PranavVadrevu restored the get_by_key/get_by_key_with_limits branch December 11, 2023 14:12
@PranavVadrevu PranavVadrevu reopened this Dec 11, 2023
@chrismccord
Copy link
Member

Can we get a rundown on the rationale and needs for this feature? The issue with opening up the lookup interfaces is we are rather tied to what we can do inside ets in the future. It may be fine, but it's something we need to be careful with, so I'd like to know more about what specifically you are needing. Thanks!

@PranavVadrevu
Copy link
Author

Can we get a rundown on the rationale and needs for this feature? The issue with opening up the lookup interfaces is we are rather tied to what we can do inside ets in the future. It may be fine, but it's something we need to be careful with, so I'd like to know more about what specifically you are needing. Thanks!

Hey @chrismccord, I'm sorry - I meant to push to our custom fork of the repo, didn't mean to get this PR in the main Phoenix Framework, sorry about that 😅

@indrekj
Copy link
Contributor

indrekj commented Jan 3, 2024

@PranavVadrevu may be useful for you: #127 We've been using this for a long time now.

@PranavVadrevu
Copy link
Author

@PranavVadrevu may be useful for you: #127 We've been using this for a long time now.

Thanks a lot @indrekj, it looks like it should definitely help!

@PranavVadrevu
Copy link
Author

Tiny question @indrekj, did you do a performance comparison between dirty_list and your solution? If the main optimization to be made is to remove the Genserver blocking call (which dirty_list also does), wondering if this solution adds more efficiency (if we don't care whether the list is dirty or not)

@indrekj
Copy link
Contributor

indrekj commented Jan 3, 2024

I don't remember exactly anymore. It's been almost 5 years :). I think our main goal was to improve performance while keeping the existing guarantees and interface. The GenServer part was the major blocker. An extra call to down_replicas ets which is generally very small is likely a very minor optimization.

@PranavVadrevu
Copy link
Author

I see, that's great to know, thank you!

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.

6 participants