uschindler commented on pull request #2052:
URL: https://github.com/apache/lucene-solr/pull/2052#issuecomment-748528098


   Hi, I rewrote most of the ByteBuffer handling. The bugs came from the fact 
that we used an additional outside bufferSize and bufferPos. All calcuations 
were done based on this, and the fields inside the ByteBuffer got completely 
out of sync, if alignment was wrong.
   
   Instead of doing all this, I applied the standard ByteBuffer workflows as 
documented ("how to use a ByteBuffer with flip, reset, remaining,...). The new 
code uses ByteBuffer.hasRemaining() and remaining() to check if there's space. 
The buffer size is buffer.limit() and the alignment in constructor is done with 
overallocation. The code is now much cleaner, no strange assers and masks 
anymore. The seek() method now aligns at real blockSize of OS device (using a 
modulo operation).
   
   Nevertheless, testing is a bit bad, as the default BaseDirectoryTestcase 
methods often use the delegate, as the MERGE mode is done seldom. We should 
maybe extend BaseDirectoryTestcase to use a "non-random" default IOContext for 
all tests. I can do this in a later step.
   
   In addition, there is no test using a real index. When using this directory 
in reality it will bail out, as this method is not implemented!!!!
   
   ```java
       @Override
       public long getChecksum() {
         throw new UnsupportedOperationException("this directory currently does 
not work at all!");
       }
   ```
   
   We have to implement checksums!


----------------------------------------------------------------
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:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to