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 2022/10/06 09:04:10 UTC

[GitHub] [accumulo-testing] keith-turner opened a new pull request, #236: fixes bug with bulk RW file partition point creation

keith-turner opened a new pull request, #236:
URL: https://github.com/apache/accumulo-testing/pull/236

   There was code that did the following
   
       TreeSet<Integer> startRows = new TreeSet<>();
       startRows.add(0);
       while (startRows.size() < parts)
         startRows.add(rand.nextInt(LOTS));
   
   The above code was replaced in 7453c37 with a stream. The stream did not fully capture the original behavior of the loop.  This change makes the stream fully capture that behavior.  Need to ensure that `parts` unique random numbers are generated including zero (like if the random number generator returns zero it should be properly deduplicated).  The stream was not properly handling the RNG returning duplicates or zero.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [accumulo-testing] keith-turner commented on pull request #236: fixes bug with bulk RW file partition point creation

Posted by GitBox <gi...@apache.org>.
keith-turner commented on PR #236:
URL: https://github.com/apache/accumulo-testing/pull/236#issuecomment-1269650250

   > I'm beginning to wonder if we should replace the stream with the original loop. The created set must always have at least 0 in it. Will the stream changes I created with Stream.concat result in this?
   
   I looked at the stream javadoc and it will maintain order, so since `0` comes first it should always be there.
   
   https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/stream/Stream.html#concat(java.util.stream.Stream,java.util.stream.Stream)
   
   I think the stream code may be slightly more readable than the loop with the explicit call to distinct(). So I think its slightly better than the loop.  I am going to add a comment about zero.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [accumulo-testing] keith-turner commented on pull request #236: fixes bug with bulk RW file partition point creation

Posted by GitBox <gi...@apache.org>.
keith-turner commented on PR #236:
URL: https://github.com/apache/accumulo-testing/pull/236#issuecomment-1269641554

   I'm beginning to wonder if we should replace the stream with the original loop.  The created set must always have at least `0` in it.  Will the stream changes I created with Stream.concat result in this?


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [accumulo-testing] keith-turner commented on pull request #236: fixes bug with bulk RW file partition point creation

Posted by GitBox <gi...@apache.org>.
keith-turner commented on PR #236:
URL: https://github.com/apache/accumulo-testing/pull/236#issuecomment-1269662527

   I lost the exception, but I was seeing error when run the Bulk RW test.  When duplicate rand nums were generated it would create a set that was too small and this would result in an index out bounds exception on a later loop.  This PR is in support of running bulk RW test for apache/accumulo#2667.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [accumulo-testing] keith-turner merged pull request #236: fixes bug with bulk RW file partition point creation

Posted by GitBox <gi...@apache.org>.
keith-turner merged PR #236:
URL: https://github.com/apache/accumulo-testing/pull/236


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org