You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/11/13 23:01:33 UTC
[GitHub] [accumulo] ctubbsii opened a new pull request #1779: Stabilize NamespaceIdTest and TableIdTest
ctubbsii opened a new pull request #1779:
URL: https://github.com/apache/accumulo/pull/1779
TableId and NamespaceId have tests to ensure their caches work
correctly. Previously, the tests would only *attempt* to increase the
cache before checking that it decreased again. However, a garbage
collection could have reduced the size of the cache before we checked
its size. This change *guarantees* the cache will reach a specific size
before proceeding.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a change in pull request #1779: Stabilize NamespaceIdTest and TableIdTest
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1779:
URL: https://github.com/apache/accumulo/pull/1779#discussion_r524453508
##########
File path: core/src/test/java/org/apache/accumulo/core/data/NamespaceIdTest.java
##########
@@ -79,12 +79,13 @@ public void testCacheIncreasesAndDecreasesAfterGC() {
assertEquals(namespaceString, nsId.canonical());
// create a bunch more and throw them away
- for (int i = 0; i < 999; i++) {
- NamespaceId.of(new String("namespace" + i));
+ long preGCSize = 0;
+ int i = 0;
+ while ((preGCSize = NamespaceId.cache.asMap().entrySet().stream().count()) < 100) {
Review comment:
Ah thank you. I was racking my brain trying to figure out the difference between the two.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1779: Stabilize NamespaceIdTest and TableIdTest
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1779:
URL: https://github.com/apache/accumulo/pull/1779#discussion_r524440754
##########
File path: core/src/test/java/org/apache/accumulo/core/data/TableIdTest.java
##########
@@ -88,12 +88,13 @@ public void testCacheIncreasesAndDecreasesAfterGC() {
assertEquals(tableString, table1.canonical());
// create a bunch more and throw them away
- for (int i = 0; i < 999; i++) {
- TableId.of(new String("table" + i));
+ long preGCSize = 0;
+ int i = 0;
+ while ((preGCSize = TableId.cache.asMap().entrySet().stream().count()) < 100) {
Review comment:
Yeah, the IDE is wrong in this case. See my other comment.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1779: Stabilize NamespaceIdTest and TableIdTest
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1779:
URL: https://github.com/apache/accumulo/pull/1779#discussion_r524440237
##########
File path: core/src/test/java/org/apache/accumulo/core/data/NamespaceIdTest.java
##########
@@ -79,12 +79,13 @@ public void testCacheIncreasesAndDecreasesAfterGC() {
assertEquals(namespaceString, nsId.canonical());
// create a bunch more and throw them away
- for (int i = 0; i < 999; i++) {
- NamespaceId.of(new String("namespace" + i));
+ long preGCSize = 0;
+ int i = 0;
+ while ((preGCSize = NamespaceId.cache.asMap().entrySet().stream().count()) < 100) {
Review comment:
We can't use `size()` (either on the `Map` or its `entrySet()`), because that is only an approximation when working with Guava Cache objects. We need the actual count, which we can get by iterating over the entry set. See https://guava.dev/releases/21.0/api/docs/com/google/common/cache/Cache.html#size--
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on pull request #1779: Stabilize NamespaceIdTest and TableIdTest
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #1779:
URL: https://github.com/apache/accumulo/pull/1779#issuecomment-728217355
@ctubbsii looking at these tests led me to open this ticket to examine this cache further: https://github.com/apache/accumulo/issues/1781
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a change in pull request #1779: Stabilize NamespaceIdTest and TableIdTest
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1779:
URL: https://github.com/apache/accumulo/pull/1779#discussion_r524434634
##########
File path: core/src/test/java/org/apache/accumulo/core/data/NamespaceIdTest.java
##########
@@ -79,12 +79,13 @@ public void testCacheIncreasesAndDecreasesAfterGC() {
assertEquals(namespaceString, nsId.canonical());
// create a bunch more and throw them away
- for (int i = 0; i < 999; i++) {
- NamespaceId.of(new String("namespace" + i));
+ long preGCSize = 0;
+ int i = 0;
+ while ((preGCSize = NamespaceId.cache.asMap().entrySet().stream().count()) < 100) {
Review comment:
```suggestion
while ((preGCSize = NamespaceId.cache.asMap().entrySet().size()) < 100) {
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii merged pull request #1779: Stabilize NamespaceIdTest and TableIdTest
Posted by GitBox <gi...@apache.org>.
ctubbsii merged pull request #1779:
URL: https://github.com/apache/accumulo/pull/1779
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a change in pull request #1779: Stabilize NamespaceIdTest and TableIdTest
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1779:
URL: https://github.com/apache/accumulo/pull/1779#discussion_r524437783
##########
File path: core/src/test/java/org/apache/accumulo/core/data/TableIdTest.java
##########
@@ -88,12 +88,13 @@ public void testCacheIncreasesAndDecreasesAfterGC() {
assertEquals(tableString, table1.canonical());
// create a bunch more and throw them away
- for (int i = 0; i < 999; i++) {
- TableId.of(new String("table" + i));
+ long preGCSize = 0;
+ int i = 0;
+ while ((preGCSize = TableId.cache.asMap().entrySet().stream().count()) < 100) {
Review comment:
This was a suggestion from my IDE but I don't think they are exactly the same...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a change in pull request #1779: Stabilize NamespaceIdTest and TableIdTest
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1779:
URL: https://github.com/apache/accumulo/pull/1779#discussion_r524435438
##########
File path: core/src/test/java/org/apache/accumulo/core/data/TableIdTest.java
##########
@@ -88,12 +88,13 @@ public void testCacheIncreasesAndDecreasesAfterGC() {
assertEquals(tableString, table1.canonical());
// create a bunch more and throw them away
- for (int i = 0; i < 999; i++) {
- TableId.of(new String("table" + i));
+ long preGCSize = 0;
+ int i = 0;
+ while ((preGCSize = TableId.cache.asMap().entrySet().stream().count()) < 100) {
Review comment:
```suggestion
while ((preGCSize = TableId.cache.asMap().entrySet().size()) < 100) {
```
There are also many other occurances in the test you could replace.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org