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.

Sign up for GitHub

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

Jump to bottom

Add entry stats #184

Merged
merged 11 commits into from
Nov 5, 2019
Merged

Add entry stats #184

merged 11 commits into from
Nov 5, 2019

Conversation

Nazdroth
Copy link
Contributor

@Nazdroth Nazdroth commented Nov 4, 2019

This pull request resolves #159.

  • Adding a way to access entry-level stats, that can be enabled/disabled using the config attribute StatsEnabled which is set to false by default.
  • Adding a way to get metadata related to a particular key using KeyMetaData(key string)
  • OnRemove now can give you the possibility to access the entry metadata using OnRemoveWithMetaData.
  • Writing unit tests for the added functions.

@siennathesane
Copy link
Collaborator

@cristaloleg do you know what happened to the checks?

siennathesane
siennathesane requested changes Nov 4, 2019
View reviewed changes
Copy link
Collaborator

@siennathesane siennathesane left a comment

Choose a reason for hiding this comment

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

Looks great, only minor feedback.

bigcache.go Outdated Show resolved Hide resolved
bigcache.go Outdated Show resolved Hide resolved
config.go Show resolved Hide resolved
config.go Outdated Show resolved Hide resolved
config.go Show resolved Hide resolved
shard.go Outdated Show resolved Hide resolved
shard.go Show resolved Hide resolved
shard.go Outdated Show resolved Hide resolved
janisz
janisz previously approved these changes Nov 4, 2019
View reviewed changes
siennathesane
siennathesane previously approved these changes Nov 4, 2019
View reviewed changes
Copy link
Collaborator

@siennathesane siennathesane left a comment

Choose a reason for hiding this comment

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

LGTM, I'm fine with this. 👍

cristaloleg
cristaloleg requested changes Nov 5, 2019
View reviewed changes
Copy link
Collaborator

@cristaloleg cristaloleg left a comment

Choose a reason for hiding this comment

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

Nice! SGTM, but I've few comments.

shard.go Outdated Show resolved Hide resolved
shard.go Outdated Show resolved Hide resolved
@Nazdroth Nazdroth dismissed stale reviews from siennathesane and janisz via 10ddb0b November 5, 2019 08:45
cristaloleg
cristaloleg approved these changes Nov 5, 2019
View reviewed changes
Copy link
Collaborator

@cristaloleg cristaloleg left a comment

Choose a reason for hiding this comment

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

Well done! Merging

@cristaloleg cristaloleg changed the title Issue #159 Add entry stats Nov 5, 2019
@cristaloleg cristaloleg merged commit 4945ddf into allegro:master Nov 5, 2019
@Nazdroth Nazdroth deleted the feat/entryStats branch November 5, 2019 17:27
flisky pushed a commit to flisky/bigcache that referenced this pull request May 7, 2020
@Nazdroth @cristaloleg
Add entry stats (allegro#184)
f723661 
Fixes allegro#159
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cristaloleg cristaloleg cristaloleg approved these changes

@janisz janisz janisz left review comments

@siennathesane siennathesane siennathesane left review comments

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Entry stats
4 participants
@Nazdroth @siennathesane @cristaloleg @janisz