Hi Greg, Thanks for your response. 1. Some changes will be made to the Compression and Builder interfaces. At least one default method, isAvailable(), would be added to the Builder interface. The static methods in the Compression interface that create and return specific builders would be modified to return a new builder or a loaded builder from a service provider if it exists. The return types may need to be more generic e.g Builder<GzipCompression> vs GzipCompression.Builder. 2. These classes are appropriate to add to the API surface as they may be required to develop efficient compression implementations. I will modify the KIP. 4. This is because the Compression interface (currently) expects a Builder of type 'GzipCompression.Builder'. I agree that alternative implementations need not extend the base implementation with some modifications to the Compression interface. However if an alternative implementation is registered as a service provider of GzipCompression then it must be of type 'CompressionType.GZIP' and it must be fully compatible with the base implementation of GzipCompression. 5. So far no, but 'Configurable' looks promising 6. If no specific configuration is set, then the first provider loaded and available will be used. If a specific configuration is set then that specific provider will be used if available else the base implementation. 7. Yes, a ConfigException will be thrown if the class specified in the config is not found on the classpath. 8. This can be implemented by using special filtering or a custom class loader. The concern here is that any class on the class path (trusted or not) could be loaded which could intercept messages or corrupt messages by compressing using an unknown algorithm.
-----Original Message----- From: Greg Harris <greg.har...@aiven.io.INVALID> Sent: Tuesday, February 18, 2025 1:58 PM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-1121: Compression acceleration in Kafka Hi Olasoji, Thanks for the KIP! I find it addresses many of the concerns I had with KIP-984. Thank you for following up on that! 1. Since the Compression interface is internal currently, it may contain some components that were not designed with pluggability in mind, such as the static methods. Are these interfaces being published as-is, or are some changes going to be made first? 2. Similarly, the ByteBufferOutputStream, CompressionType, BufferSupplier, ByteBufferInputStream, ChunkedByteStream, and GzipOutputStream classes in the signature or implementations of Compression are not public API currently, and would need to be made public along with Compression. Are these classes appropriate to add to our API surface? 3. Could you adjust the KIP structure/formatting to make the public API surface area more clear? Some prose in "Proposed Changes" could become code examples in "Public Interfaces". 4. What is the significance of this statement: "Any new service provider should extend the base implementation and thus return the same CompressionType". Are we expecting that `o instanceof GzipCompression == o.type().equals(CompressionType.GZIP)` at all times? If an alternative implementation doesn't need any part of the upstream implementation, does it make sense to force them to extend one another? 5. Is this interface expected to be composed with any other plugin mix-ins, such as Configurable, Reconfigurable, Closeable, Monitorable, Versioned, etc? 6. If there are multiple accelerators present, which one will be chosen? 7. If the configuration is set but the accelerator implementation isn't available, does that cause a ConfigException? 8. Could you expand on this statement: "Due to security and/or compatibility concerns it is possible to limit this feature to only load approved service providers (e.g those distributed with Kafka)". How will this be implemented? What threat model does this assume? Thanks, Greg On Tue, Feb 18, 2025 at 11:41 AM Denloye, Olasoji <olasoji.denl...@intel.com> wrote: > Hi, I would like to bring this back to everyone's attention. This KIP > introduces a simple framework that enables the use of alternative, > faster implementations of the existing compression codecs (Gzip, > Snappy, Zstandard and LZ4) in Kafka. > > https://cwiki.apache.org/confluence/x/Xgr0Ew > > Thanks > > -----Original Message----- > From: Denloye, Olasoji <olasoji.denl...@intel.com> > Sent: Thursday, February 13, 2025 1:22 PM > To: dev@kafka.apache.org > Subject: RE: [DISCUSS] KIP-1121: Compression acceleration in Kafka > > Hi, I would like to bring this back to everyone's attention. I have > made changes to the proposal to incorporate the ServiceLoader API as > suggested. > > https://cwiki.apache.org/confluence/x/Xgr0Ew > > Thanks > > -----Original Message----- > From: Denloye, Olasoji > Sent: Thursday, December 19, 2024 11:01 AM > To: dev@kafka.apache.org > Subject: RE: [DISCUSS] KIP-1121: Compression acceleration in Kafka > > Hi Frédérik, Thanks for the suggestion to use the existing > ServiceLoader feature in Java. It could serve as suitable mechanism > for selecting the best available compression codec. > > Thanks > > -----Original Message----- > From: Frédérik Rouleau <froul...@confluent.io.INVALID> > Sent: Wednesday, December 18, 2024 12:24 AM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-1121: Compression acceleration in Kafka > > Hi Olasoji, > > I think that having a way to provide some optimised/accelerated > implementation is a good idea. > What not using the ServiceLoader feature of Java? That would allow > anyone to provide their own optimised implementation depending on > their infrastructure. We can keep the existing implementation as the > default and allow Kafka to find a better implementation based on > Service found. As you mentioned, we might need to enrich the interface > to allow Kafka to find the best version available at runtime. > > Regards, > > On Tue, Dec 17, 2024 at 6:36 PM Denloye, Olasoji < > olasoji.denl...@intel.com> > wrote: > > > Hi Everyone, > > > > I would like to start a discussion on KIP-1121: Compression > > acceleration acceleration in Kafka. KIP-1121: Compression > > acceleration in Kafka - Apache Kafka - Apache Software Foundation< > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1121%3A+Compre > > ss > > ion+acceleration+in+Kafka > > > > > >
