-
Notifications
You must be signed in to change notification settings - Fork 3
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
Fixed LookupMetadata hit/miss counts #100
Conversation
initialize a new (or *many* new) LookupMetadata instances.
Codecov Report
@@ Coverage Diff @@
## master #100 +/- ##
===========================================
+ Coverage 87.91% 89.61% +1.7%
- Complexity 684 719 +35
===========================================
Files 52 56 +4
Lines 2259 2678 +419
Branches 171 176 +5
===========================================
+ Hits 1986 2400 +414
- Misses 185 193 +8
+ Partials 88 85 -3
Continue to review full report at Codecov.
|
I'll start working on the test cases now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great start - testing tomorrow!
if (luckyMe) { | ||
result = loadMetadata(); | ||
// a reloadInterval of 0 prevents reloading of the metadata | ||
boolean reloadMetadata = !reloadStamp.compareAndSet(stamp[0], stamp[0] + reloadInterval); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's test this a bunch.
We need to figure out how to hit both the elapsed time condition and the first thread to compare and set...
that all calls to findKey are properly accounted.
for LookupData.getMetadata().
mocked instances. Also inverted the reloadMetadata flag in the getMetadata method.
write(pos, byteRecord(bytes)); | ||
if (log.isTraceEnabled()) log.trace("appended {} bytes to {} at pos {}", bytes.length, virtualFileNumber, pos); | ||
blobStoreMetricsAdders.appendCounter.increment(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How much might using these counters affect performance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious to see what total time is for LongAdder.increment in JProfiler...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment in the code, but I see no problems here fwiw.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we are there!
Added the ability to use an existing LookupMetadata instance to initialize a new (or many new) LookupMetadata instances.