accumulo icon indicating copy to clipboard operation
accumulo copied to clipboard

LRUCache is Evicting Too Many CachedBlocks

Open belugabehr opened this issue 6 years ago • 7 comments

Prefer ArrayList over LinkedList

https://stackoverflow.com/questions/322715/when-to-use-linkedlist-over-arraylist-in-java

Also, instead of having to drain the queue in a manual way, simply grab all the elements and call 'clear' on the queue since calling 'remove' on a PriorityQueue is O(log(n)) time anyway.

https://docs.oracle.com/javase/7/docs/api/java/util/PriorityQueue.html

belugabehr avatar Aug 23 '19 18:08 belugabehr

@belugabehr I'm not going to be able to continue following the issue this week, because I'm at ApacheCon right now and my spare time is thin. So, please don't take my silence as disinterest. I can probably revisit this again next week, unless somebody else is able to review and help you wrap this up before then.

Also, I was thinking: because this issue's objectives seem to have evolved a bit from the original goal of replacing LinkedList with ArrayList, it might be a useful exercise to take a step back and identify your current (and specific) objective (and state it explicitly here), before continuing work... just so it's clear what is being worked towards here.

ctubbsii avatar Sep 12 '19 06:09 ctubbsii

@ctubbsii Are you able to take another look at this PR? Thanks!

belugabehr avatar Sep 17 '19 18:09 belugabehr

@ctubbsii Are you able to take another look at this PR? Thanks!

Possibly later today (Friday). Still playing catch-up from being at ApacheCon last week.

ctubbsii avatar Sep 20 '19 04:09 ctubbsii

@ctubbsii Any possibility to re-examine this at your next bug bash session?

In particular, please check out:

https://github.com/apache/accumulo/pull/1333#discussion_r323728667

belugabehr avatar Oct 02 '19 14:10 belugabehr

@ctubbsii Any possibility to re-examine this at your next bug bash session?

In particular, please check out:

#1333 (comment)

Once again, sorry for the delay. I had family issues shortly after returning from ApacheCon. This is still near the top of my TODO list. I've completely lost context for this change, so I'll have to go back and read the comments to try to remember where this was at.

In the meantime, if another committer can review, that would probably help a ton. I recommend introducing yourself on the developer mailing list, and pinging other devs, to see if they can offer a review. I don't want to be a bottleneck or roadblock.

ctubbsii avatar Oct 05 '19 00:10 ctubbsii

@phrocker @keith-turner Thank you so much for looking over. I made the requested changes. The only thing is that I wasn't sure what to do with that one unit test that was validating perhaps some sort of historical behavior about "must always maintain heapSize >= maxSize once we achieve it." I marked that test as 'Ignored' for now.

belugabehr avatar Oct 28 '19 18:10 belugabehr

@belugabehr This issue has crept a bit from its starting point. I'd like to refocus with a question: Do you have a concrete end goal in mind here, and is that goal important to you?

I ask if it's important to you because my impression is that you're trying to get more involved, but that these specific code changes have stalled (on the side of the reviewers) due to being high-risk and low-reward (because the code is complicated and the goal has gotten fuzzy). I understand that stall can make you feel discouraged, which we definitely don't want to do.

So, if it is important to you, perhaps there's some way you can define your overall goal more concretely so that others can understand and so that you can make progress incrementally towards it. But, if this area of the code is not that important to you, then it might be a good idea to abandon the work because the risk-reward tradeoff isn't worth it, and for us to help encourage you contributing elsewhere. I'm okay with either direction, but currently, I worry this issue isn't headed in a direction that makes the best use of your time.

ctubbsii avatar Nov 06 '19 05:11 ctubbsii