Skip to content

feat: add value iterator - #475

Merged
matthewmcneely merged 1 commit into
dgraph-io:mainfrom
wundergraph:milinda/iterating_elements
Jan 21, 2026
Merged

matthewmcneely merged 1 commit into
dgraph-io:mainfrom
wundergraph:milinda/iterating_elements

Conversation

@SkArchon

@SkArchon SkArchon commented Jan 13, 2026 •

Copy link
Copy Markdown
Contributor

Description

This PR is based off of the PR here #460, I have opened a new PR as this only contains a subset of the changes, and I do not have access to push to the original authors fork.

This PR only contains the iteration function, which iterates on values.

cc: @ilyatotl, @mangalaman93

Checklist

  • Code compiles correctly and linting passes locally
  • Tests added for new functionality, or regression tests for bug fixes added as applicable

@SkArchon
SkArchon requested a review from a team as a code owner January 13, 2026 15:08
@CLAassistant

CLAassistant commented Jan 13, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@SkArchon

SkArchon commented Jan 13, 2026 •

Copy link
Copy Markdown
Contributor Author

@mangalaman93 Please do let me know if this PR is sufficient for merged based on your earlier comment here #460 (review).

I'm assuming based on the CLAssistant, @ilyatotl will need to sign the CLA, if not I might have to squash his commits.

@SkArchon SkArchon changed the title fix: updates fix: iterate on elements Jan 13, 2026
@matthewmcneely

Copy link
Copy Markdown
Collaborator

@SkArchon While I can appreciate the usefulness of a first class iterator, I'm worried about the increase in overall memory usage this PR will introduce. 16 bytes for the OriginalKey field, which represents a 20% increase in memory use. Not trivial. My guess is this is why the original authors left it out.

Could you not roll-your-own by embedding a Cache inside your own struct? Implement Set and Get and store the keys in a Map, then provide the Iter func from there?

@SkArchon

SkArchon commented Jan 16, 2026 •

Copy link
Copy Markdown
Contributor Author

Hi @matthewmcneely, I am happy to remove the OriginalKey attribute, as we don't need it for our use case, I left it in because nothing was mentioned regarding it by the previous reviewer in the original PR. Will this work for you? If so I can update the PR. I would assume I could rename the function it into IterValues.

For context, we want to repopulate the ristretto cache upon restarts of our router (from the previous instance of the ristretto cache), so with evictions and such using the actual ristretto cache makes sense since it will be the source of truth. But on top of this we would also prefer to not do this if possible by having another map and lock on it for Set and Get wrappers since its on the hot path.

@matthewmcneely

Copy link
Copy Markdown
Collaborator

@SkArchon Happy to review a version without the addition of an Item field.

@SkArchon
SkArchon force-pushed the milinda/iterating_elements branch 2 times, most recently from 21be33a to 5112a10 Compare January 18, 2026 19:17
@SkArchon

Copy link
Copy Markdown
Contributor Author

@matthewmcneely I have updated the PR to have iterations only on values, please do let me know if there are any changes to be made, I also squashed the commit history due to the original author being inactive to accept the CLA.

@matthewmcneely matthewmcneely left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looking better, thanks!

Comment thread cache.go
Comment thread store.go Outdated
Comment thread cache_test.go Outdated
@SkArchon

Copy link
Copy Markdown
Contributor Author

Also do let me know if you would like me to mark the conversations as resolved, or to wait for you the reviewer to mark it as resolved.

@matthewmcneely matthewmcneely changed the title fix: iterate on elements feat: add value iterator Jan 19, 2026
@SkArchon

Copy link
Copy Markdown
Contributor Author

Hi @matthewmcneely, I see that you have resolved your previous review comments, please do let me know what else is needed to get this merged.

@matthewmcneely

Copy link
Copy Markdown
Collaborator

@SkArchon Can you rebase your fork? I cannot resolve conflicts on forks.

@SkArchon
SkArchon force-pushed the milinda/iterating_elements branch from f52c09a to fb3ddb3 Compare January 20, 2026 19:53
@SkArchon

Copy link
Copy Markdown
Contributor Author

Because of the merge commit I found it easier to squash. Anyway the merge-base is the tip of the main branch now.

@matthewmcneely
matthewmcneely merged commit 3e164e4 into dgraph-io:main Jan 21, 2026
2 checks passed
@SkArchon

Copy link
Copy Markdown
Contributor Author

@matthewmcneely Quick question, would it be possible to have a ristretto release (so this would be included), I see the last release in August 2025.

@matthewmcneely

Copy link
Copy Markdown
Collaborator

@SkArchon We released v2.4.0 yesterday. Thanks for your contributions.

gamesguru pushed a commit to gamesguru/dendrite that referenced this pull request Aug 27, 2026
…ent-hq#21)

This PR contains the following updates:

| Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) |
|---|---|---|---|
| [github.com/dgraph-io/ristretto](https://github.com/dgraph-io/ristretto) | `v0.2.0` → `v2.4.0` | ![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fdgraph-io%2fristretto/v2.4.0?slim=true) | ![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fdgraph-io%2fristretto/v0.2.0/v2.4.0?slim=true) |

---

> ⚠️ **Warning**
>
> Some dependencies could not be looked up. Check the Dependency Dashboard for more information.

---

### Release Notes

<details>
<summary>dgraph-io/ristretto (github.com/dgraph-io/ristretto)</summary>

### [`v2.4.0`](https://github.com/dgraph-io/ristretto/blob/HEAD/CHANGELOG.md#v240---2026-01-21)

[Compare Source](dgraph-io/ristretto@v2.3.0...v2.4.0)

##### Added

- Implement public `Cache.IterValues()` method ([#&#8203;475](dgraph-io/ristretto#475))
- Allow custom key types with underlying types in Key constraint ([#&#8203;478](dgraph-io/ristretto#478))

##### Fixed

- Fix compilation on 32-bit archs ([#&#8203;465](dgraph-io/ristretto#465))

**Full Changelog**: <dgraph-io/ristretto@v2.3.0...v2.4.0>

### [`v2.3.0`](https://github.com/dgraph-io/ristretto/blob/HEAD/CHANGELOG.md#v240---2026-01-21)

[Compare Source](dgraph-io/ristretto@v2.2.0...v2.3.0)

##### Added

- Implement public `Cache.IterValues()` method ([#&#8203;475](dgraph-io/ristretto#475))
- Allow custom key types with underlying types in Key constraint ([#&#8203;478](dgraph-io/ristretto#478))

##### Fixed

- Fix compilation on 32-bit archs ([#&#8203;465](dgraph-io/ristretto#465))

**Full Changelog**: <dgraph-io/ristretto@v2.3.0...v2.4.0>

### [`v2.2.0`](https://github.com/dgraph-io/ristretto/blob/HEAD/CHANGELOG.md#v230---2025-08-19)

[Compare Source](dgraph-io/ristretto@v2.1.0...v2.2.0)

##### Added

- Add public `Cache.RemainingCost()` method ([#&#8203;448](dgraph-io/ristretto#448))
- Add support for uint keys ([#&#8203;463](dgraph-io/ristretto#463))

##### Fixed

- Fix typo: ffor → for ([#&#8203;456](dgraph-io/ristretto#456))
- Correct grammar in error message ([#&#8203;461](dgraph-io/ristretto#461))

**Full Changelog**: <dgraph-io/ristretto@v2.2.0...v2.3.0>

### [`v2.1.0`](https://github.com/dgraph-io/ristretto/blob/HEAD/CHANGELOG.md#v220---2025-03-30)

[Compare Source](dgraph-io/ristretto@v2.0.1...v2.1.0)

##### Changed

- Remove dependency: github.com/pkg/errors ([#&#8203;443](dgraph-io/ristretto#443))

##### Fixed

- Switch from using a sync.WaitGroup to closing a channel of struct{} ([#&#8203;442](dgraph-io/ristretto#442))

**Full Changelog**: <dgraph-io/ristretto@v2.1.0...v2.2.0>

### [`v2.0.1`](https://github.com/dgraph-io/ristretto/blob/HEAD/CHANGELOG.md#v210---2025-01-09)

[Compare Source](dgraph-io/ristretto@v2.0.0...v2.0.1)

##### Added

- Add `ShouldUpdate()` function in config ([#&#8203;427](dgraph-io/ristretto#427))

##### Fixed

- Fix memory leak while cleaning up expiration map ([#&#8203;429](dgraph-io/ristretto#429))
- Execute `m.Unlock` in defer in store.go ([#&#8203;425](dgraph-io/ristretto#425))

**Full Changelog**: <dgraph-io/ristretto@v2.0.1...v2.1.0>

### [`v2.0.0`](https://github.com/dgraph-io/ristretto/blob/HEAD/CHANGELOG.md#v201---2024-12-11)

[Compare Source](dgraph-io/ristretto@v1.0.1...v2.0.0)

**Fixed**

- Wait for goroutines to finish ([#&#8203;423](dgraph-io/ristretto#423))
- Bump golang.org/x/sys from 0.27.0 to 0.28.0 in the minor group ([#&#8203;421](dgraph-io/ristretto#421))
- Bump github.com/stretchr/testify from 1.9.0 to 1.10.0 in the minor group ([#&#8203;420](dgraph-io/ristretto#420))
- Bump golang.org/x/sys from 0.26.0 to 0.27.0 in the minor group ([#&#8203;419](dgraph-io/ristretto#419))

**Full Changelog**: <dgraph-io/ristretto@v2.0.0...v2.0.1>

### [`v1.0.1`](https://github.com/dgraph-io/ristretto/blob/HEAD/CHANGELOG.md#v101)

[Compare Source](dgraph-io/ristretto@v1.0.0...v1.0.1)

**This release is deprecated**

### [`v1.0.0`](https://github.com/dgraph-io/ristretto/blob/HEAD/CHANGELOG.md#v100)

[Compare Source](dgraph-io/ristretto@v0.2.0...v1.0.0)

**This release is deprecated**

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "every 2 weeks on monday" (UTC), Automerge - Between 12:00 AM and 03:59 AM ( * 0-3 * * * ) (UTC).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4wLjYiLCJ1cGRhdGVkSW5WZXIiOiI0My40LjAiLCJ0YXJnZXRCcmFuY2giOiJtYWluIiwibGFiZWxzIjpbXX0=-->

Co-authored-by: pat-s <patrick.schratz@gmail.com>
Reviewed-on: https://codefloe.com/pat-s/dendrite/pulls/21
Co-authored-by: renovate-bot <renovate@codefloe.com>
Co-committed-by: renovate-bot <renovate@codefloe.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants