Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Peeking at compressed messages throws an exception (Readonly buffers not supported by Airlift) #8974

Closed
eolivelli opened this issue Dec 16, 2020 · 4 comments · Fixed by #8990
Closed

Peeking at compressed messages throws an exception (Readonly buffers not supported by Airlift) #8974

eolivelli opened this issue Dec 16, 2020 · 4 comments · Fixed by #8990
Labels
release/2.6.3 type/bug The PR fixed a bug or issue reported a bug

Comments

@eolivelli
Copy link
Contributor

Earlier in the 2.6.0 branch, the compression library was swapped for https://github.com/airlift/aircompressor.

When messages are published with LZ4 compression and you try to use the Peek API, https://pulsar.apache.org/admin/v2/persistent/{tenant}/{namespace}/{topic}/subscription/{subName}/position/{messagePosition} on the message, the broker throws an exception, causing a 500 return code. This breaks the dashboard.

Here is the exception:


14:54:25.006 [pulsar-web-43-1] ERROR org.apache.pulsar.broker.admin.impl.PersistentTopicsBase - [superuser] Failed to get message at position 1 from persistent://xxxxxx
java.lang.IllegalArgumentException: Unsupported input ByteBuffer implementation java.nio.HeapByteBufferR
        at io.airlift.compress.lz4.Lz4Decompressor.decompress(Lz4Decompressor.java:58) ~[io.airlift-aircompressor-0.16.jar:0.16]
        at org.apache.pulsar.common.compression.CompressionCodecLZ4.decode(CompressionCodecLZ4.java:100) ~[org.apache.pulsar-pulsar-common-2.6.1.jar:2.6.1]
        at org.apache.pulsar.broker.admin.impl.PersistentTopicsBase.generateResponseWithEntry(PersistentTopicsBase.java:1977) ~[org.apache.pulsar-pulsar-broker-2.6.1.jar
:2.6.1]
        at org.apache.pulsar.broker.admin.impl.PersistentTopicsBase.internalPeekNthMessage(PersistentTopicsBase.java:1921) ~[org.apache.pulsar-pulsar-broker-2.6.1.jar:2.
6.1]

If I recall correctly, this also fails for SNAPPY compression, but works fine for other compression types (ZLIB).

Here is the pull request where this change was introduced: #5390

I am 100% certain this is the cause of the issue. Reverting this PR resolves the issue.

Some potentially useful references:

See:
https://github.com/airlift/aircompressor/blob/24b4f18dcb8a70d273ba1d1cd4c0715f15909573/src/main/java/io/airlift/compress/lz4/Lz4Decompressor.java#L58
aeron-io/agrona#66 (comment)
@eolivelli eolivelli added the type/bug The PR fixed a bug or issue reported a bug label Dec 16, 2020
@eolivelli
Copy link
Contributor Author

This is a regression on 2.6 release line
@wolfstudy @codelipenghui please consider this problem before releasing 2.6.3

@sijie sijie mentioned this issue Dec 16, 2020
Closed
@Jennifer88huang-zz
Copy link
Contributor

It seems that some PR introduced issue...

@merlimat
Copy link
Contributor

The problem seems to be the bytebuffer is wrapped and is neither "direct" or it has an array. We should be probably taking a copy of that buffer to solve this.

@eolivelli
Copy link
Contributor Author

@merlimat I am working on a fix, thanks for the advice

@eolivelli eolivelli changed the title Peeking at compressed messages throws an exception Peeking at compressed messages throws an exception (Readonly buffers not supported by Airlift) Dec 17, 2020
@eolivelli eolivelli mentioned this issue Dec 17, 2020
Merged
sijie pushed a commit that referenced this issue Dec 22, 2020
@eolivelli
Issue 8974: Peeking at compressed messages throws an exception (Reado…
cbc606b 
…nly buffers not supported by Airlift) (#8990)

Fixes #8974 

### Motivation
In certain cases peeking messages on compresses topics return an error, see #8974 because Airlift does not support readonly ByteBuffers, because they do not give access to the underlying array)

### Modifications

Copy the ByteByffer in case of unsupported buffer type

### Verifying this change

This change adds new tests that reproduce the error and demonstrate that the problem is fixed.
codelipenghui pushed a commit that referenced this issue Dec 23, 2020
@eolivelli @codelipenghui
Issue 8974: Peeking at compressed messages throws an exception (Reado…
fbbc251 
…nly buffers not supported by Airlift) (#8990)

Fixes #8974 

### Motivation
In certain cases peeking messages on compresses topics return an error, see #8974 because Airlift does not support readonly ByteBuffers, because they do not give access to the underlying array)

### Modifications

Copy the ByteByffer in case of unsupported buffer type

### Verifying this change

This change adds new tests that reproduce the error and demonstrate that the problem is fixed.

(cherry picked from commit cbc606b)
codelipenghui pushed a commit that referenced this issue Dec 23, 2020
@eolivelli @codelipenghui
Issue 8974: Peeking at compressed messages throws an exception (Reado…
057b33e 
…nly buffers not supported by Airlift) (#8990)

Fixes #8974 

### Motivation
In certain cases peeking messages on compresses topics return an error, see #8974 because Airlift does not support readonly ByteBuffers, because they do not give access to the underlying array)

### Modifications

Copy the ByteByffer in case of unsupported buffer type

### Verifying this change

This change adds new tests that reproduce the error and demonstrate that the problem is fixed.

(cherry picked from commit cbc606b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
release/2.6.3 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Milestone
No milestone
3 participants
@merlimat @eolivelli @Jennifer88huang-zz