You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@pinot.apache.org by "gortiz (via GitHub)" <gi...@apache.org> on 2023/09/25 08:38:39 UTC

[GitHub] [pinot] gortiz opened a new pull request, #11672: Java 21 pipeline

gortiz opened a new pull request, #11672:
URL: https://github.com/apache/pinot/pull/11672

   The reason to be of this PR is to be able to progress on https://github.com/apache/pinot/issues/11656.
   
   Specifically it:
   - Includes https://github.com/apache/pinot/pull/11670
   - Includes https://github.com/apache/pinot/pull/11671
   - Adds 2 new tests pipelines:
     - One with Java 21.
     - One with Java 21 that always use the new buffer api. Normally this API is only used when buffers are larger than 2GBs. The idea of this execution is to always use this API (even with small buffers) to increase the confidence on the library.


-- 
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: commits-unsubscribe@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Java 21 pipeline [pinot]

Posted by "gortiz (via GitHub)" <gi...@apache.org>.

gortiz commented on code in PR #11672:
URL: https://github.com/apache/pinot/pull/11672#discussion_r1360318921


##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/memory/PinotDataBuffer.java:
##########
@@ -152,17 +153,22 @@ public static PinotBufferFactory getFactory() {
   }
 
   public static PinotBufferFactory createDefaultFactory() {
-    return createDefaultFactory(true);
+    String skipBbEnvValue = System.getenv(SKIP_BYTEBUFFER_ENV);
+    boolean prioritizeByteBuffer = !Boolean.parseBoolean(skipBbEnvValue);

Review Comment:
   Changed.
   
   But remember this is called only once per process in production code, so there is no actual gain.



-- 
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: commits-unsubscribe@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Java 21 pipeline [pinot]

Posted by "gortiz (via GitHub)" <gi...@apache.org>.

gortiz commented on PR #11672:
URL: https://github.com/apache/pinot/pull/11672#issuecomment-1746381234

   > I think we can just test the default byteBuffer usage.
   > For the others, if you have good tests, we can just write a test suite for testing different behaviors to ensure the coverage.
   
   `PinotDataBufferTest` already does that... but as is proven by this PR, it is not enough. There are some particularities in LArray that are not tested there. Given the importance of the buffer library (it may imply incorrect results or data corruption), I think it is worth it to have at least one execution where the unsafe buffer is tested.
   
   But we can skip it if you think it is not necessary


-- 
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: commits-unsubscribe@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org