From ab45e0d474cee1a8b642c6564da7dce369d6e648 Mon Sep 17 00:00:00 2001 From: Sunjeet Singh Date: Wed, 21 Jun 2023 15:26:13 -0700 Subject: [PATCH] Change unexpected read from exception to warn --- .../hollow/api/client/HollowDataHolder.java | 2 +- .../hollow/core/memory/EncodedByteBuffer.java | 7 ++-- .../core/memory/encoding/BlobByteBuffer.java | 36 +++++++++---------- ...eapArrayVsOffHeapBufferAcceptanceTest.java | 18 +++++----- 4 files changed, 31 insertions(+), 32 deletions(-) diff --git a/hollow/src/main/java/com/netflix/hollow/api/client/HollowDataHolder.java b/hollow/src/main/java/com/netflix/hollow/api/client/HollowDataHolder.java index e473ffb320..8e2f1a6a02 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/client/HollowDataHolder.java +++ b/hollow/src/main/java/com/netflix/hollow/api/client/HollowDataHolder.java @@ -208,7 +208,7 @@ private void applyDeltaOnlyPlan(HollowUpdatePlan updatePlan, HollowConsumer.Refr private void applyDeltaTransition(HollowConsumer.Blob blob, boolean isSnapshotPlan, HollowConsumer.RefreshListener[] refreshListeners) throws Throwable { LOG.info("Attempting delta transition ..."); if (!memoryMode.equals(MemoryMode.ON_HEAP)) { - LOG.info("SNAP: Attempting delta transition in shared-memory mode ..."); + LOG.info(String.format("SNAP: Attempting delta transition to %s in shared-memory mode ...", blob.getToVersion())); } try (HollowBlobInput in = HollowBlobInput.modeBasedSelector(memoryMode, blob); diff --git a/hollow/src/main/java/com/netflix/hollow/core/memory/EncodedByteBuffer.java b/hollow/src/main/java/com/netflix/hollow/core/memory/EncodedByteBuffer.java index b6a78fda24..e1b3560efd 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/memory/EncodedByteBuffer.java +++ b/hollow/src/main/java/com/netflix/hollow/core/memory/EncodedByteBuffer.java @@ -63,10 +63,11 @@ public void destroy() throws IOException { @Override public byte get(long index) { - if (index >= this.size) { - throw new IllegalStateException(); + if (index >= this.size) { // SNAP: TODO: realized in transformer + if (index >= this.size + Long.BYTES) { + LOG.warning(String.format("SNAP: unexpected get from EncodedByteBuffer: index=%s, size=%s", index, size)); + } } - byte retVal = this.bufferView.getByte(this.bufferView.position() + index); return retVal; } diff --git a/hollow/src/main/java/com/netflix/hollow/core/memory/encoding/BlobByteBuffer.java b/hollow/src/main/java/com/netflix/hollow/core/memory/encoding/BlobByteBuffer.java index 184770e74b..386fed6775 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/memory/encoding/BlobByteBuffer.java +++ b/hollow/src/main/java/com/netflix/hollow/core/memory/encoding/BlobByteBuffer.java @@ -164,30 +164,28 @@ public byte getByte(long index) throws BufferUnderflowException { // advances pos in backing buf public int getBytes(long index, long len, byte[] bytes, boolean restorePos) { - if (index < capacity) { - int spineIndex = (int)(index >>> (shift)); - ByteBuffer buf = spine[spineIndex]; - int indexIntoBuf = (int)(index & mask); - int toCopy = (int) Math.min(len, buf.capacity() - indexIntoBuf); - int savePos = buf.position(); - try { - buf.position(indexIntoBuf); - buf.get(bytes, 0, toCopy); - if (restorePos) { - buf.position(savePos); - } - } catch (BufferUnderflowException e) { - throw e; - } - return toCopy; - } else { - assert(index < capacity + Long.BYTES); + if (index >= capacity) { // this situation occurs when read for bits near the end of the buffer requires reading a long value that // extends past the buffer capacity by upto Long.BYTES bytes. To handle this case, // return 0 for (index >= capacity - Long.BYTES && index < capacity ) // these zero bytes will be discarded anyway when the returned long value is shifted to get the queried bits - throw new UnsupportedOperationException(String.format("Unexpected read past the end, index=%s, capacity=%s", index, capacity)); + LOG.warning(String.format("Unexpected read past the end, index=%s, capacity=%s", index, capacity)); + } + int spineIndex = (int)(index >>> (shift)); + ByteBuffer buf = spine[spineIndex]; + int indexIntoBuf = (int)(index & mask); + int toCopy = (int) Math.min(len, buf.capacity() - indexIntoBuf); + int savePos = buf.position(); + try { + buf.position(indexIntoBuf); + buf.get(bytes, 0, toCopy); + if (restorePos) { + buf.position(savePos); + } + } catch (BufferUnderflowException e) { + throw e; } + return toCopy; } public int putBytes(long index, long len, byte[] bytes, boolean restorePos) { diff --git a/hollow/src/test/java/com/netflix/hollow/core/memory/encoding/OnHeapArrayVsOffHeapBufferAcceptanceTest.java b/hollow/src/test/java/com/netflix/hollow/core/memory/encoding/OnHeapArrayVsOffHeapBufferAcceptanceTest.java index 6360540854..8237bf0f78 100644 --- a/hollow/src/test/java/com/netflix/hollow/core/memory/encoding/OnHeapArrayVsOffHeapBufferAcceptanceTest.java +++ b/hollow/src/test/java/com/netflix/hollow/core/memory/encoding/OnHeapArrayVsOffHeapBufferAcceptanceTest.java @@ -137,15 +137,15 @@ private void readUsingVariableLengthDataModes(File testFile, int padding) throws assertEquals(testByteArray.get(13 + padding), testByteBuffer.get(13 + padding)); assertEquals(testByteArray.get(127 + padding), testByteBuffer.get(127 + padding)); - // out of bounds read - try { - testByteBuffer.get(testFile.length()); - Assert.fail(); - } catch (IllegalStateException e) { - // this is expected - } catch (Exception e) { - Assert.fail(); - } + // SNAP: TODO: // out of bounds read + // try { + // testByteBuffer.get(testFile.length()); + // Assert.fail(); + // } catch (IllegalStateException e) { + // // this is expected + // } catch (Exception e) { + // Assert.fail(); + // } } // write a File of TEST_SINGLE_BUFFER_CAPACITY_BYTES*4 size, assuming TEST_SINGLE_BUFFER_CAPACITY_BYTES is 32