Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
chia7712 commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2921770908
##
build.gradle:
##
@@ -2046,6 +2046,7 @@ project(':clients') {
include "**/org/apache/kafka/common/security/scram/*"
include "**/org/apache/kafka/common/security/token/delegation/*"
include "**/org/apache/kafka/common/serialization/*"
+include "**/org/apache/kafka/common/utils/Bytes.java"
Review Comment:
opened https://issues.apache.org/jira/browse/KAFKA-20297
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
chia7712 merged PR #21601: URL: https://github.com/apache/kafka/pull/21601 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
chia7712 commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2921758097
##
build.gradle:
##
@@ -2046,6 +2046,7 @@ project(':clients') {
include "**/org/apache/kafka/common/security/scram/*"
include "**/org/apache/kafka/common/security/token/delegation/*"
include "**/org/apache/kafka/common/serialization/*"
+include "**/org/apache/kafka/common/utils/Bytes.java"
Review Comment:
I'm going to merge this PR. The cleanup of other classes in
`org/apache/kafka/common/utils` will be handled in the follow-up PRs
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on PR #21601: URL: https://github.com/apache/kafka/pull/21601#issuecomment-4042299233 Hi @chia7712 @mjsax, With code freeze coming up on March 18, wanted to check on the status of this PR. I believe I have addressed all feedback so far. Is there anything else needed for approval? Ready to make any quick adjustments if needed. Thank you -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2897775664
##
clients/src/main/java/org/apache/kafka/common/utils/internals/BytesUtils.java:
##
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.common.utils.internals;
+
+import org.apache.kafka.common.utils.Bytes;
+
+import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Comparator;
+
+/**
+ * Internal utility class for Bytes-related operations.
+ * This class is for internal Kafka use only and is not part of the public API.
+ */
+public final class BytesUtils {
+
+private BytesUtils() {
+// Utility class, prevent instantiation
+}
+
+/**
+ * Increment the underlying byte array by adding 1.
+ *
+ * @param input - The byte array to increment
+ * @return A new copy of the incremented byte array
+ * @throws IndexOutOfBoundsException if incrementing causes the underlying
input byte array to overflow
+ */
+public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
+byte[] inputArr = input.get();
+byte[] ret = new byte[inputArr.length];
+int carry = 1;
+for (int i = inputArr.length - 1; i >= 0; i--) {
+if (inputArr[i] == (byte) 0xFF && carry == 1) {
+ret[i] = (byte) 0x00;
+} else {
+ret[i] = (byte) (inputArr[i] + carry);
+carry = 0;
+}
+}
+if (carry == 0) {
+return Bytes.wrap(ret);
+} else {
+throw new IndexOutOfBoundsException();
+}
+}
+
+/**
+ * A byte array comparator based on lexicographic ordering.
+ */
+public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
+
+public interface ByteArrayComparator extends Comparator,
Serializable {
Review Comment:
@chia7712 - I removed Serializable from the internals `ByteArrayComparator`
interface as you suggested.
SpotBugs was complaining about SE_COMPARATOR_SHOULD_BE_SERIALIZABLE for the
LexicographicByteArrayComparator class, so I added an exclusion in
gradle/spotbugs-exclude.xml since this is internal code not intended for
serialization.
The deprecated public interface still keeps Serializable for backward
compatibility.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2897775664
##
clients/src/main/java/org/apache/kafka/common/utils/internals/BytesUtils.java:
##
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.common.utils.internals;
+
+import org.apache.kafka.common.utils.Bytes;
+
+import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Comparator;
+
+/**
+ * Internal utility class for Bytes-related operations.
+ * This class is for internal Kafka use only and is not part of the public API.
+ */
+public final class BytesUtils {
+
+private BytesUtils() {
+// Utility class, prevent instantiation
+}
+
+/**
+ * Increment the underlying byte array by adding 1.
+ *
+ * @param input - The byte array to increment
+ * @return A new copy of the incremented byte array
+ * @throws IndexOutOfBoundsException if incrementing causes the underlying
input byte array to overflow
+ */
+public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
+byte[] inputArr = input.get();
+byte[] ret = new byte[inputArr.length];
+int carry = 1;
+for (int i = inputArr.length - 1; i >= 0; i--) {
+if (inputArr[i] == (byte) 0xFF && carry == 1) {
+ret[i] = (byte) 0x00;
+} else {
+ret[i] = (byte) (inputArr[i] + carry);
+carry = 0;
+}
+}
+if (carry == 0) {
+return Bytes.wrap(ret);
+} else {
+throw new IndexOutOfBoundsException();
+}
+}
+
+/**
+ * A byte array comparator based on lexicographic ordering.
+ */
+public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
+
+public interface ByteArrayComparator extends Comparator,
Serializable {
Review Comment:
@chia7712 - I removed Serializable from the internals ByteArrayComparator as
you suggested, but now SpotBugs is failing with
SE_COMPARATOR_SHOULD_BE_SERIALIZABLE:
> "This class implements the Comparator interface. You should consider
whether or not it should also implement the Serializable interface. As most
comparators have little or no state, making them serializable is generally easy
and good defensive programming."
I searched the codebase for `@SuppressFBWarnings` or similar SpotBugs
suppression annotations but couldn't find any existing usage, so I am not sure
what the preferred approach is.
Should I:
- Keep Serializable in the internals version (for SpotBugs/defensive
programming), or
- Handle the SpotBugs warning some other way?
What's the standard practice for Kafka when SpotBugs and design decisions
conflict?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2897775664
##
clients/src/main/java/org/apache/kafka/common/utils/internals/BytesUtils.java:
##
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.common.utils.internals;
+
+import org.apache.kafka.common.utils.Bytes;
+
+import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Comparator;
+
+/**
+ * Internal utility class for Bytes-related operations.
+ * This class is for internal Kafka use only and is not part of the public API.
+ */
+public final class BytesUtils {
+
+private BytesUtils() {
+// Utility class, prevent instantiation
+}
+
+/**
+ * Increment the underlying byte array by adding 1.
+ *
+ * @param input - The byte array to increment
+ * @return A new copy of the incremented byte array
+ * @throws IndexOutOfBoundsException if incrementing causes the underlying
input byte array to overflow
+ */
+public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
+byte[] inputArr = input.get();
+byte[] ret = new byte[inputArr.length];
+int carry = 1;
+for (int i = inputArr.length - 1; i >= 0; i--) {
+if (inputArr[i] == (byte) 0xFF && carry == 1) {
+ret[i] = (byte) 0x00;
+} else {
+ret[i] = (byte) (inputArr[i] + carry);
+carry = 0;
+}
+}
+if (carry == 0) {
+return Bytes.wrap(ret);
+} else {
+throw new IndexOutOfBoundsException();
+}
+}
+
+/**
+ * A byte array comparator based on lexicographic ordering.
+ */
+public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
+
+public interface ByteArrayComparator extends Comparator,
Serializable {
Review Comment:
@chia7712 - I removed Serializable from the internals ByteArrayComparator as
you suggested, but now SpotBugs is failing with
SE_COMPARATOR_SHOULD_BE_SERIALIZABLE:
> "This class implements the Comparator interface. You should consider
whether or not it should also implement the Serializable interface. As most
comparators have little or no state, making them serializable is generally easy
and good defensive programming."
Should I:
- Keep Serializable in the internals version (for SpotBugs/defensive
programming), or
- Remove it and configure SpotBugs to ignore this warning for internal
classes?
What's the preferred approach for Kafka?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2897092071
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -34,6 +43,14 @@ public class Bytes implements Comparable {
// cache the hash code for the string, default to 0
private int hashCode;
+/**
+ * Creates a Bytes instance wrapping the given byte array.
+ *
+ * The provided array becomes the backing storage for the object.
+ *
+ * @param bytes the byte array to wrap, or null
+ * @return a new Bytes instance, or null if the input is null
+ */
public static Bytes wrap(byte[] bytes) {
if (bytes == null)
Review Comment:
Yeah, right, my apologies as i overlooked the `bytes.length` in the caller
method.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2897118611
##
clients/src/main/java/org/apache/kafka/common/utils/internals/BytesUtils.java:
##
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.common.utils.internals;
+
+import org.apache.kafka.common.utils.Bytes;
+
+import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Comparator;
+
+/**
+ * Internal utility class for Bytes-related operations.
+ * This class is for internal Kafka use only and is not part of the public API.
+ */
+public final class BytesUtils {
+
+private BytesUtils() {
+// Utility class, prevent instantiation
+}
+
+/**
+ * Increment the underlying byte array by adding 1.
+ *
+ * @param input - The byte array to increment
+ * @return A new copy of the incremented byte array
+ * @throws IndexOutOfBoundsException if incrementing causes the underlying
input byte array to overflow
+ */
+public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
+byte[] inputArr = input.get();
+byte[] ret = new byte[inputArr.length];
+int carry = 1;
+for (int i = inputArr.length - 1; i >= 0; i--) {
+if (inputArr[i] == (byte) 0xFF && carry == 1) {
+ret[i] = (byte) 0x00;
+} else {
+ret[i] = (byte) (inputArr[i] + carry);
+carry = 0;
+}
+}
+if (carry == 0) {
+return Bytes.wrap(ret);
+} else {
+throw new IndexOutOfBoundsException();
+}
+}
+
+/**
+ * A byte array comparator based on lexicographic ordering.
+ */
+public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
+
+public interface ByteArrayComparator extends Comparator,
Serializable {
Review Comment:
You are right again, we don't need it. The backward compatibility is
maintained through the deprecated public `Bytes.ByteArrayComparator` interface,
but same is not the case for internals interface.
Thank you Chia.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2897092071
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -34,6 +43,14 @@ public class Bytes implements Comparable {
// cache the hash code for the string, default to 0
private int hashCode;
+/**
+ * Creates a Bytes instance wrapping the given byte array.
+ *
+ * The provided array becomes the backing storage for the object.
+ *
+ * @param bytes the byte array to wrap, or null
+ * @return a new Bytes instance, or null if the input is null
+ */
public static Bytes wrap(byte[] bytes) {
if (bytes == null)
Review Comment:
Yeah, right, my apologies as i overlooked the `bytes.length`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
chia7712 commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2894214114
##
clients/src/main/java/org/apache/kafka/common/utils/internals/BytesUtils.java:
##
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.common.utils.internals;
+
+import org.apache.kafka.common.utils.Bytes;
+
+import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Comparator;
+
+/**
+ * Internal utility class for Bytes-related operations.
+ * This class is for internal Kafka use only and is not part of the public API.
+ */
+public final class BytesUtils {
+
+private BytesUtils() {
+// Utility class, prevent instantiation
+}
+
+/**
+ * Increment the underlying byte array by adding 1.
+ *
+ * @param input - The byte array to increment
+ * @return A new copy of the incremented byte array
+ * @throws IndexOutOfBoundsException if incrementing causes the underlying
input byte array to overflow
+ */
+public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
+byte[] inputArr = input.get();
+byte[] ret = new byte[inputArr.length];
+int carry = 1;
+for (int i = inputArr.length - 1; i >= 0; i--) {
+if (inputArr[i] == (byte) 0xFF && carry == 1) {
+ret[i] = (byte) 0x00;
+} else {
+ret[i] = (byte) (inputArr[i] + carry);
+carry = 0;
+}
+}
+if (carry == 0) {
+return Bytes.wrap(ret);
+} else {
+throw new IndexOutOfBoundsException();
+}
+}
+
+/**
+ * A byte array comparator based on lexicographic ordering.
+ */
+public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
+
+public interface ByteArrayComparator extends Comparator,
Serializable {
Review Comment:
Is implementing `Serializable` still necessary here? I don't think this
class is intended for java's native serialization
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
chia7712 commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2894158001
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -34,6 +43,14 @@ public class Bytes implements Comparable {
// cache the hash code for the string, default to 0
private int hashCode;
+/**
+ * Creates a Bytes instance wrapping the given byte array.
+ *
+ * The provided array becomes the backing storage for the object.
+ *
+ * @param bytes the byte array to wrap, or null
+ * @return a new Bytes instance, or null if the input is null
+ */
public static Bytes wrap(byte[] bytes) {
if (bytes == null)
Review Comment:
```java
@Override
public String toString() {
return Bytes.toString(bytes, 0, bytes.length);
}
```
`bytes.length` could cause NPE
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2892481257
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -34,6 +43,14 @@ public class Bytes implements Comparable {
// cache the hash code for the string, default to 0
private int hashCode;
+/**
+ * Creates a Bytes instance wrapping the given byte array.
+ *
+ * The provided array becomes the backing storage for the object.
+ *
+ * @param bytes the byte array to wrap, or null
+ * @return a new Bytes instance, or null if the input is null
+ */
public static Bytes wrap(byte[] bytes) {
if (bytes == null)
Review Comment:
Thank you Chia, you are right on the inconsistency of null handling.
Actually, `toString()` does handle null - the private `toString(byte[], int,
int)` method has a null check:
```java
private static String toString(final byte[] b, int off, int len) {
if (b == null)
return result.toString(); // Returns empty string
```
But the method `compareTo()` will throw an NPE if the internal `bytes` array
is null, since the comparator accesses `bytes.length` without any null check.
I have added a null check to the constructor using
`Objects.requireNonNull()` so that:
- `wrap(null)` continues to return null (backward compatible)
- `new Bytes(null)` throws an NPE immediately (fail fast)
- All Bytes instances are guaranteed to have a non-null bytes array
- No null checks needed in methods like `compareTo()`
This makes it now consistent i guess?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
chia7712 commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2889547717
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -34,6 +43,14 @@ public class Bytes implements Comparable {
// cache the hash code for the string, default to 0
private int hashCode;
+/**
+ * Creates a Bytes instance wrapping the given byte array.
+ *
+ * The provided array becomes the backing storage for the object.
+ *
+ * @param bytes the byte array to wrap, or null
+ * @return a new Bytes instance, or null if the input is null
+ */
public static Bytes wrap(byte[] bytes) {
if (bytes == null)
Review Comment:
Does `Bytes` constructor accept a `null` input? If not, we should document
this behaviour and add a strict null check. If it does, we need to handle
`null` in all methods. For instance, `toString` would currently throw an NPE
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2874691847
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
Review Comment:
fixed, please resolve the thread if you are okay with it, thanks.
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,7 +163,10 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
Review Comment:
fixed, please resolve the thread if you are okay with it, thanks.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2874694528
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +163,36 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
-byte[] inputArr = input.get();
-byte[] ret = new byte[inputArr.length];
-int carry = 1;
-for (int i = inputArr.length - 1; i >= 0; i--) {
-if (inputArr[i] == (byte) 0xFF && carry == 1) {
-ret[i] = (byte) 0x00;
-} else {
-ret[i] = (byte) (inputArr[i] + carry);
-carry = 0;
-}
-}
-if (carry == 0) {
-return wrap(ret);
-} else {
-throw new IndexOutOfBoundsException();
-}
+return BytesUtils.increment(input);
}
/**
* A byte array comparator based on lexicograpic ordering.
Review Comment:
fixed, please resolve the thread if you are okay with it, thanks.
##
clients/src/main/java/org/apache/kafka/common/utils/internals/BytesUtils.java:
##
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.common.utils.internals;
+
+import org.apache.kafka.common.utils.Bytes;
+
+import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Comparator;
+
+/**
+ * Internal utility class for Bytes-related operations.
+ * This class is for internal Kafka use only and is not part of the public API.
+ */
+public final class BytesUtils {
+
+private BytesUtils() {
+// Utility class, prevent instantiation
+}
+
+/**
+ * Increment the underlying byte array by adding 1. Throws an
IndexOutOfBoundsException if incrementing would cause
Review Comment:
fixed, please resolve the thread if you are okay with it, thanks.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2874692914
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +161,32 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
-byte[] inputArr = input.get();
-byte[] ret = new byte[inputArr.length];
-int carry = 1;
-for (int i = inputArr.length - 1; i >= 0; i--) {
-if (inputArr[i] == (byte) 0xFF && carry == 1) {
-ret[i] = (byte) 0x00;
-} else {
-ret[i] = (byte) (inputArr[i] + carry);
-carry = 0;
-}
-}
-if (carry == 0) {
-return wrap(ret);
-} else {
-throw new IndexOutOfBoundsException();
-}
+return BytesUtils.increment(input);
}
/**
* A byte array comparator based on lexicograpic ordering.
+ * @deprecated This field is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#BYTES_LEXICO_COMPARATOR}
instead.
*/
-public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
+@Deprecated(since = "4.3", forRemoval = true)
+public static final BytesUtils.ByteArrayComparator BYTES_LEXICO_COMPARATOR
= new LexicographicByteArrayComparator();
-public interface ByteArrayComparator extends Comparator,
Serializable {
-
-int compare(final byte[] buffer1, int offset1, int length1,
-final byte[] buffer2, int offset2, int length2);
+/**
+ * A byte array comparator interface.
+ *
+ * @deprecated This interface is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils.ByteArrayComparator} instead.
+ */
+@Deprecated(since = "4.3", forRemoval = true)
+public interface ByteArrayComparator extends
BytesUtils.ByteArrayComparator {
}
-private static class LexicographicByteArrayComparator implements
ByteArrayComparator {
-
-@Override
-public int compare(byte[] buffer1, byte[] buffer2) {
-return compare(buffer1, 0, buffer1.length, buffer2, 0,
buffer2.length);
-}
-
-public int compare(final byte[] buffer1, int offset1, int length1,
- final byte[] buffer2, int offset2, int length2) {
-
-// short circuit equal case
-if (buffer1 == buffer2 &&
-offset1 == offset2 &&
-length1 == length2) {
-return 0;
-}
-
-int end1 = offset1 + length1;
-int end2 = offset2 + length2;
-return Arrays.compareUnsigned(buffer1, offset1, end1, buffer2,
offset2, end2);
-}
+private static class LexicographicByteArrayComparator extends
BytesUtils.LexicographicByteArrayComparator {
Review Comment:
fixed, please resolve the thread if you are okay with it, thanks.
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +161,32 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
-byte[] inputArr = input.get();
-byte[] ret = new byte[inputArr.length];
-int carry = 1;
-for (int i = inputArr.length - 1; i >= 0; i--) {
-if (inputArr[i] == (byte) 0xFF && carry == 1) {
-ret[i] = (byte) 0x00;
-} else {
-ret[i] = (byte) (inputArr[i] + carry);
-carry = 0;
-}
-}
-if (carry == 0) {
-return wrap(ret);
-} else {
-throw new IndexOutOfBoundsException();
-}
+return BytesUtils.increment(input);
}
/**
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2874694322
##
clients/src/main/java/org/apache/kafka/common/utils/internals/BytesUtils.java:
##
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.common.utils.internals;
+
+import org.apache.kafka.common.utils.Bytes;
+
+import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Comparator;
+
+/**
+ * Internal utility class for Bytes-related operations.
+ * This class is for internal Kafka use only and is not part of the public API.
+ */
+public final class BytesUtils {
+
+private BytesUtils() {
+// Utility class, prevent instantiation
+}
+
+/**
+ * Increment the underlying byte array by adding 1. Throws an
IndexOutOfBoundsException if incrementing would cause
+ * the underlying input byte array to overflow.
+ *
+ * @param input - The byte array to increment
+ * @return A new copy of the incremented byte array.
+ */
+public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
+byte[] inputArr = input.get();
+byte[] ret = new byte[inputArr.length];
+int carry = 1;
+for (int i = inputArr.length - 1; i >= 0; i--) {
+if (inputArr[i] == (byte) 0xFF && carry == 1) {
+ret[i] = (byte) 0x00;
+} else {
+ret[i] = (byte) (inputArr[i] + carry);
+carry = 0;
+}
+}
+if (carry == 0) {
+return Bytes.wrap(ret);
+} else {
+throw new IndexOutOfBoundsException();
+}
+}
+
+/**
+ * A byte array comparator based on lexicograpic ordering.
Review Comment:
fixed, please resolve the thread if you are okay with it, thanks.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2874693786
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +163,36 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
-byte[] inputArr = input.get();
-byte[] ret = new byte[inputArr.length];
-int carry = 1;
-for (int i = inputArr.length - 1; i >= 0; i--) {
-if (inputArr[i] == (byte) 0xFF && carry == 1) {
-ret[i] = (byte) 0x00;
-} else {
-ret[i] = (byte) (inputArr[i] + carry);
-carry = 0;
-}
-}
-if (carry == 0) {
-return wrap(ret);
-} else {
-throw new IndexOutOfBoundsException();
-}
+return BytesUtils.increment(input);
}
/**
* A byte array comparator based on lexicograpic ordering.
+ * @deprecated This field is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#BYTES_LEXICO_COMPARATOR}
instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
+/**
+ * A byte array comparator interface.
+ *
+ * @deprecated This interface is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils.ByteArrayComparator} instead.
+ */
+@Deprecated(since = "4.3", forRemoval = true)
public interface ByteArrayComparator extends Comparator,
Serializable {
int compare(final byte[] buffer1, int offset1, int length1,
final byte[] buffer2, int offset2, int length2);
}
-private static class LexicographicByteArrayComparator implements
ByteArrayComparator {
-
-@Override
-public int compare(byte[] buffer1, byte[] buffer2) {
-return compare(buffer1, 0, buffer1.length, buffer2, 0,
buffer2.length);
-}
-
-public int compare(final byte[] buffer1, int offset1, int length1,
- final byte[] buffer2, int offset2, int length2) {
-
-// short circuit equal case
-if (buffer1 == buffer2 &&
-offset1 == offset2 &&
-length1 == length2) {
-return 0;
-}
-
-int end1 = offset1 + length1;
-int end2 = offset2 + length2;
-return Arrays.compareUnsigned(buffer1, offset1, end1, buffer2,
offset2, end2);
-}
+private static class LexicographicByteArrayComparator extends
BytesUtils.LexicographicByteArrayComparator implements ByteArrayComparator {
+// Empty - inherits implementation from BytesUtils, but explicitly
declares it implements the local interface
Review Comment:
fixed, please resolve the thread if you are okay with it, thanks.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2880403957
##
build.gradle:
##
@@ -2046,6 +2046,7 @@ project(':clients') {
include "**/org/apache/kafka/common/security/scram/*"
include "**/org/apache/kafka/common/security/token/delegation/*"
include "**/org/apache/kafka/common/serialization/*"
+include "**/org/apache/kafka/common/utils/Bytes.java"
Review Comment:
Done, please check
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
chia7712 commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2876719551
##
build.gradle:
##
@@ -2046,6 +2046,7 @@ project(':clients') {
include "**/org/apache/kafka/common/security/scram/*"
include "**/org/apache/kafka/common/security/token/delegation/*"
include "**/org/apache/kafka/common/serialization/*"
+include "**/org/apache/kafka/common/utils/Bytes.java"
Review Comment:
Make sense. Let's stick with `include
"**/org/apache/kafka/common/utils/Bytes.java` and leave `Time`, `Timer`, and
`Bytes` in their original package. That could minimize the surprise. Also, it
would be helpful to add comments to `package-info.java` mentioning the KIP that
will discuss the exposition of the `Time` and `Timer` classes. WDYT?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2874691097
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,7 +163,10 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
Review Comment:
fixed
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
Review Comment:
fixed
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2874693786
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +163,36 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
-byte[] inputArr = input.get();
-byte[] ret = new byte[inputArr.length];
-int carry = 1;
-for (int i = inputArr.length - 1; i >= 0; i--) {
-if (inputArr[i] == (byte) 0xFF && carry == 1) {
-ret[i] = (byte) 0x00;
-} else {
-ret[i] = (byte) (inputArr[i] + carry);
-carry = 0;
-}
-}
-if (carry == 0) {
-return wrap(ret);
-} else {
-throw new IndexOutOfBoundsException();
-}
+return BytesUtils.increment(input);
}
/**
* A byte array comparator based on lexicograpic ordering.
+ * @deprecated This field is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#BYTES_LEXICO_COMPARATOR}
instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
+/**
+ * A byte array comparator interface.
+ *
+ * @deprecated This interface is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils.ByteArrayComparator} instead.
+ */
+@Deprecated(since = "4.3", forRemoval = true)
public interface ByteArrayComparator extends Comparator,
Serializable {
int compare(final byte[] buffer1, int offset1, int length1,
final byte[] buffer2, int offset2, int length2);
}
-private static class LexicographicByteArrayComparator implements
ByteArrayComparator {
-
-@Override
-public int compare(byte[] buffer1, byte[] buffer2) {
-return compare(buffer1, 0, buffer1.length, buffer2, 0,
buffer2.length);
-}
-
-public int compare(final byte[] buffer1, int offset1, int length1,
- final byte[] buffer2, int offset2, int length2) {
-
-// short circuit equal case
-if (buffer1 == buffer2 &&
-offset1 == offset2 &&
-length1 == length2) {
-return 0;
-}
-
-int end1 = offset1 + length1;
-int end2 = offset2 + length2;
-return Arrays.compareUnsigned(buffer1, offset1, end1, buffer2,
offset2, end2);
-}
+private static class LexicographicByteArrayComparator extends
BytesUtils.LexicographicByteArrayComparator implements ByteArrayComparator {
+// Empty - inherits implementation from BytesUtils, but explicitly
declares it implements the local interface
Review Comment:
fixed
##
clients/src/main/java/org/apache/kafka/common/utils/internals/BytesUtils.java:
##
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.common.utils.internals;
+
+import org.apache.kafka.common.utils.Bytes;
+
+import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Comparator;
+
+/**
+ * Internal utility class for Bytes-related operations.
+ * This class is for internal Kafka use only and is not part of the public API.
+ */
+public final class BytesUtils {
+
+private BytesUtils() {
+// Utility class, prevent instantiation
+}
+
+/**
+
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2874680923
##
build.gradle:
##
@@ -2046,6 +2046,7 @@ project(':clients') {
include "**/org/apache/kafka/common/security/scram/*"
include "**/org/apache/kafka/common/security/token/delegation/*"
include "**/org/apache/kafka/common/serialization/*"
+include "**/org/apache/kafka/common/utils/Bytes.java"
Review Comment:
Fixed, thanks Chia.
But how should we proceed further?
Like Bytes, Time is exposed and since Time creates/uses Timer both these
should be part of the public APIs (in the next KIP as disussed in KIP-1247),
but we can't make them public without a KIP yet and we can't move them to the
internals package either without avoiding breaking changes since they were
already exposed (leaked) into public APIs.
If we include the entire package in the javadoc (`include
"**/org/apache/kafka/common/utils/*"`), it would make Time and Timer officially
public API without the discussion and design work that was explicitly deferred
during KIP-1247 review.
What's different from #21412 here:
The record package reorganization worked because:
- `TimestampType` was the only exposed class -> stayed where it was and
became public
- All other classes were truly internal -> moved safely
For utils:
- Multiple classes (Bytes, Time, Timer) are exposed -> can't move without
corresponding KIPs
So, considering these constraints, for now till the next KIP is accepted to
make Time and Timer public, it makes sense to include just the `Bytes.java` in
the javadoc:
```gradle
include "**/org/apache/kafka/common/utils/Bytes.java"
```
WDYT? Am I missing a simpler approach here?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2874694737
##
clients/src/main/java/org/apache/kafka/common/utils/internals/BytesUtils.java:
##
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.common.utils.internals;
+
+import org.apache.kafka.common.utils.Bytes;
+
+import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Comparator;
+
+/**
+ * Internal utility class for Bytes-related operations.
+ * This class is for internal Kafka use only and is not part of the public API.
+ */
+public final class BytesUtils {
+
+private BytesUtils() {
+// Utility class, prevent instantiation
+}
+
+/**
+ * Increment the underlying byte array by adding 1. Throws an
IndexOutOfBoundsException if incrementing would cause
Review Comment:
fixed
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2874692491
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +161,32 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
-byte[] inputArr = input.get();
-byte[] ret = new byte[inputArr.length];
-int carry = 1;
-for (int i = inputArr.length - 1; i >= 0; i--) {
-if (inputArr[i] == (byte) 0xFF && carry == 1) {
-ret[i] = (byte) 0x00;
-} else {
-ret[i] = (byte) (inputArr[i] + carry);
-carry = 0;
-}
-}
-if (carry == 0) {
-return wrap(ret);
-} else {
-throw new IndexOutOfBoundsException();
-}
+return BytesUtils.increment(input);
}
/**
* A byte array comparator based on lexicograpic ordering.
+ * @deprecated This field is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#BYTES_LEXICO_COMPARATOR}
instead.
*/
-public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
+@Deprecated(since = "4.3", forRemoval = true)
+public static final BytesUtils.ByteArrayComparator BYTES_LEXICO_COMPARATOR
= new LexicographicByteArrayComparator();
Review Comment:
fixed
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +161,32 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
-byte[] inputArr = input.get();
-byte[] ret = new byte[inputArr.length];
-int carry = 1;
-for (int i = inputArr.length - 1; i >= 0; i--) {
-if (inputArr[i] == (byte) 0xFF && carry == 1) {
-ret[i] = (byte) 0x00;
-} else {
-ret[i] = (byte) (inputArr[i] + carry);
-carry = 0;
-}
-}
-if (carry == 0) {
-return wrap(ret);
-} else {
-throw new IndexOutOfBoundsException();
-}
+return BytesUtils.increment(input);
}
/**
* A byte array comparator based on lexicograpic ordering.
+ * @deprecated This field is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#BYTES_LEXICO_COMPARATOR}
instead.
*/
-public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
+@Deprecated(since = "4.3", forRemoval = true)
+public static final BytesUtils.ByteArrayComparator BYTES_LEXICO_COMPARATOR
= new LexicographicByteArrayComparator();
-public interface ByteArrayComparator extends Comparator,
Serializable {
-
-int compare(final byte[] buffer1, int offset1, int length1,
-final byte[] buffer2, int offset2, int length2);
+/**
+ * A byte array comparator interface.
+ *
+ * @deprecated This interface is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils.ByteArrayComparator} instead.
+ */
+@Deprecated(since = "4.3", forRemoval = true)
+public interface ByteArrayComparator extends
BytesUtils.ByteArrayComparator {
Review Comment:
fixed
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +161,32 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
chia7712 commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2870807931
##
build.gradle:
##
@@ -2046,6 +2046,7 @@ project(':clients') {
include "**/org/apache/kafka/common/security/scram/*"
include "**/org/apache/kafka/common/security/token/delegation/*"
include "**/org/apache/kafka/common/serialization/*"
+include "**/org/apache/kafka/common/utils/Bytes.java"
Review Comment:
You could modify the constants defined by `MessageGenerator` to fix the
build error.
https://github.com/apache/kafka/blob/trunk/generator/src/main/java/org/apache/kafka/message/MessageGenerator.java#L87
https://github.com/apache/kafka/blob/trunk/generator/src/main/java/org/apache/kafka/message/MessageGenerator.java#L90
https://github.com/apache/kafka/blob/trunk/generator/src/main/java/org/apache/kafka/message/MessageGenerator.java#L124
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2869915268
##
build.gradle:
##
@@ -2046,6 +2046,7 @@ project(':clients') {
include "**/org/apache/kafka/common/security/scram/*"
include "**/org/apache/kafka/common/security/token/delegation/*"
include "**/org/apache/kafka/common/serialization/*"
+include "**/org/apache/kafka/common/utils/Bytes.java"
Review Comment:
I looked into following the pattern from #21412 and moving non-public
classes to an internals subpackage. However, this introduces a complication:
many of the utils classes are referenced in auto-generated code produced by the
MessageGenerator.
When I attempted the reorganization, I got compilation errors like these are
just a few of many:
Addressing this would require updating the MessageGenerator templates,
regenerating all message files, and extensive testing - significantly beyond
what was discussed in KIP-1247.
Given this complexity, what's the best path forward? Should this broader
reorganization be handled separately, or is there a simpler approach I am
missing?
./gradlew clean :clients:compileJava :clients:compileTestJava
:streams:compileJava :streams:compileTestJava
Starting a Gradle Daemon, 1 incompatible and 1 stopped Daemons could not be
reused, use --status for details
> Configure project :
Starting build with version 4.3.0-SNAPSHOT (commit id f4d3bc03) using Gradle
9.2.1, Java 21 and Scala 2.13.18
Build properties: ignoreFailures=false, maxParallelForks=10,
maxScalacThreads=8, maxTestRetries=0
> Task :streams:processMessages
MessageGenerator: processed 1 Kafka message JSON file(s).
> Task :clients:processMessages
MessageGenerator: processed 198 Kafka message JSON file(s).
> Task :clients:compileJava
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/ApiVersionsResponseData.java:41:
error: cannot find symbol
import org.apache.kafka.common.utils.ByteUtils;
^
symbol: class ByteUtils
location: package org.apache.kafka.common.utils
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/ApiVersionsResponseData.java:42:
error: cannot find symbol
import org.apache.kafka.common.utils.ImplicitLinkedHashCollection;
^
symbol: class ImplicitLinkedHashCollection
location: package org.apache.kafka.common.utils
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/ApiVersionsResponseData.java:43:
error: cannot find symbol
import org.apache.kafka.common.utils.ImplicitLinkedHashMultiCollection;
^
symbol: class ImplicitLinkedHashMultiCollection
location: package org.apache.kafka.common.utils
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/ApiVersionsResponseData.java:548:
error: package ImplicitLinkedHashMultiCollection does not exist
public static class ApiVersion implements Message,
ImplicitLinkedHashMultiCollection.Element {
^
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/LeaderChangeMessage.java:39:
error: cannot find symbol
import org.apache.kafka.common.utils.ByteUtils;
^
symbol: class ByteUtils
location: package org.apache.kafka.common.utils
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/SnapshotHeaderRecord.java:35:
error: cannot find symbol
import org.apache.kafka.common.utils.ByteUtils;
^
symbol: class ByteUtils
location: package org.apache.kafka.common.utils
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/SnapshotFooterRecord.java:35:
error: cannot find symbol
import org.apache.kafka.common.utils.ByteUtils;
^
symbol: class ByteUtils
location: package org.apache.kafka.common.utils
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/KRaftVersionRecord.java:35:
error: cannot find symbol
import org.apache.kafka.common.utils.ByteUtils;
^
symbol: class ByteUtils
location: package org.apache.kafka.common.utils
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/VotersRecord.java:41:
error: cannot find symbol
import org.apache.kafka.common.utils.ByteUtils;
^
symbol: class ByteUtils
location: package org.apache.kafka.common.utils
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2869915268
##
build.gradle:
##
@@ -2046,6 +2046,7 @@ project(':clients') {
include "**/org/apache/kafka/common/security/scram/*"
include "**/org/apache/kafka/common/security/token/delegation/*"
include "**/org/apache/kafka/common/serialization/*"
+include "**/org/apache/kafka/common/utils/Bytes.java"
Review Comment:
I looked into following the pattern from #21412 and moving non-public
classes to an internals subpackage. However, this introduces a significant
complication: many of the utils classes are referenced in auto-generated code
produced by the MessageGenerator.
When I attempted the reorganization, I got compilation errors like these are
just a few of many:
Addressing this would require updating the MessageGenerator templates,
regenerating all message files, and extensive testing - significantly beyond
what was discussed in KIP-1247.
Given this complexity, what's the best path forward? Should this broader
reorganization be handled separately, or is there a simpler approach I am
missing?
./gradlew clean :clients:compileJava :clients:compileTestJava
:streams:compileJava :streams:compileTestJava
Starting a Gradle Daemon, 1 incompatible and 1 stopped Daemons could not be
reused, use --status for details
> Configure project :
Starting build with version 4.3.0-SNAPSHOT (commit id f4d3bc03) using Gradle
9.2.1, Java 21 and Scala 2.13.18
Build properties: ignoreFailures=false, maxParallelForks=10,
maxScalacThreads=8, maxTestRetries=0
> Task :streams:processMessages
MessageGenerator: processed 1 Kafka message JSON file(s).
> Task :clients:processMessages
MessageGenerator: processed 198 Kafka message JSON file(s).
> Task :clients:compileJava
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/ApiVersionsResponseData.java:41:
error: cannot find symbol
import org.apache.kafka.common.utils.ByteUtils;
^
symbol: class ByteUtils
location: package org.apache.kafka.common.utils
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/ApiVersionsResponseData.java:42:
error: cannot find symbol
import org.apache.kafka.common.utils.ImplicitLinkedHashCollection;
^
symbol: class ImplicitLinkedHashCollection
location: package org.apache.kafka.common.utils
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/ApiVersionsResponseData.java:43:
error: cannot find symbol
import org.apache.kafka.common.utils.ImplicitLinkedHashMultiCollection;
^
symbol: class ImplicitLinkedHashMultiCollection
location: package org.apache.kafka.common.utils
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/ApiVersionsResponseData.java:548:
error: package ImplicitLinkedHashMultiCollection does not exist
public static class ApiVersion implements Message,
ImplicitLinkedHashMultiCollection.Element {
^
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/LeaderChangeMessage.java:39:
error: cannot find symbol
import org.apache.kafka.common.utils.ByteUtils;
^
symbol: class ByteUtils
location: package org.apache.kafka.common.utils
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/SnapshotHeaderRecord.java:35:
error: cannot find symbol
import org.apache.kafka.common.utils.ByteUtils;
^
symbol: class ByteUtils
location: package org.apache.kafka.common.utils
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/SnapshotFooterRecord.java:35:
error: cannot find symbol
import org.apache.kafka.common.utils.ByteUtils;
^
symbol: class ByteUtils
location: package org.apache.kafka.common.utils
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/KRaftVersionRecord.java:35:
error: cannot find symbol
import org.apache.kafka.common.utils.ByteUtils;
^
symbol: class ByteUtils
location: package org.apache.kafka.common.utils
/home/sidna/kafka/clients/build/generated/main/java/org/apache/kafka/common/message/VotersRecord.java:41:
error: cannot find symbol
import org.apache.kafka.common.utils.ByteUtils;
^
symbol: class ByteUtils
location: package org.apache.kafka.common.utils
/home/sidna/kafka/clients/build/generated/main/java/org/
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2869811676
##
build.gradle:
##
@@ -2046,6 +2046,7 @@ project(':clients') {
include "**/org/apache/kafka/common/security/scram/*"
include "**/org/apache/kafka/common/security/token/delegation/*"
include "**/org/apache/kafka/common/serialization/*"
+include "**/org/apache/kafka/common/utils/Bytes.java"
Review Comment:
You are right, it seems a bit odd.
I will move the remaining classes, except Bytes, Time and Timer into the
internals package and add the utils package to the javadoc. This would be much
cleaner. Thanks for the input.
By the way, is it "internal" or "internals"? Because there is an "internals"
package under "org.apache.kafka.streams.state",
"org.apache.kafka.common.config", "org.apache.kafka.common.header",
"org.apache.kafka.common.metrics", and many more.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2869811676
##
build.gradle:
##
@@ -2046,6 +2046,7 @@ project(':clients') {
include "**/org/apache/kafka/common/security/scram/*"
include "**/org/apache/kafka/common/security/token/delegation/*"
include "**/org/apache/kafka/common/serialization/*"
+include "**/org/apache/kafka/common/utils/Bytes.java"
Review Comment:
You are right, it seems a bit odd.
I will move the remaining classes, except Bytes and Time into the internals
package and add the utils package to the javadoc. This would be much cleaner.
Thanks for the input.
By the way, is it "internal" or "internals"? Because there is an "internals"
package under "org.apache.kafka.streams.state",
"org.apache.kafka.common.config", "org.apache.kafka.common.header",
"org.apache.kafka.common.metrics", and many more.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2869813731
##
clients/src/main/java/org/apache/kafka/common/utils/internals/BytesUtils.java:
##
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.common.utils.internals;
+
+import org.apache.kafka.common.utils.Bytes;
+
+import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Comparator;
+
+/**
+ * Internal utility class for Bytes-related operations.
+ * This class is for internal Kafka use only and is not part of the public API.
+ */
+public final class BytesUtils {
+
+private BytesUtils() {
+// Utility class, prevent instantiation
+}
+
+/**
+ * Increment the underlying byte array by adding 1. Throws an
IndexOutOfBoundsException if incrementing would cause
Review Comment:
Yeah, you are right. That makes more sense, thanks.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2869813273
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +163,36 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
-byte[] inputArr = input.get();
-byte[] ret = new byte[inputArr.length];
-int carry = 1;
-for (int i = inputArr.length - 1; i >= 0; i--) {
-if (inputArr[i] == (byte) 0xFF && carry == 1) {
-ret[i] = (byte) 0x00;
-} else {
-ret[i] = (byte) (inputArr[i] + carry);
-carry = 0;
-}
-}
-if (carry == 0) {
-return wrap(ret);
-} else {
-throw new IndexOutOfBoundsException();
-}
+return BytesUtils.increment(input);
}
/**
* A byte array comparator based on lexicograpic ordering.
Review Comment:
Thanks for the correction
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2869812701
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +163,36 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
-byte[] inputArr = input.get();
-byte[] ret = new byte[inputArr.length];
-int carry = 1;
-for (int i = inputArr.length - 1; i >= 0; i--) {
-if (inputArr[i] == (byte) 0xFF && carry == 1) {
-ret[i] = (byte) 0x00;
-} else {
-ret[i] = (byte) (inputArr[i] + carry);
-carry = 0;
-}
-}
-if (carry == 0) {
-return wrap(ret);
-} else {
-throw new IndexOutOfBoundsException();
-}
+return BytesUtils.increment(input);
}
/**
* A byte array comparator based on lexicograpic ordering.
+ * @deprecated This field is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#BYTES_LEXICO_COMPARATOR}
instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
+/**
+ * A byte array comparator interface.
+ *
+ * @deprecated This interface is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils.ByteArrayComparator} instead.
+ */
+@Deprecated(since = "4.3", forRemoval = true)
public interface ByteArrayComparator extends Comparator,
Serializable {
int compare(final byte[] buffer1, int offset1, int length1,
final byte[] buffer2, int offset2, int length2);
}
-private static class LexicographicByteArrayComparator implements
ByteArrayComparator {
-
-@Override
-public int compare(byte[] buffer1, byte[] buffer2) {
-return compare(buffer1, 0, buffer1.length, buffer2, 0,
buffer2.length);
-}
-
-public int compare(final byte[] buffer1, int offset1, int length1,
- final byte[] buffer2, int offset2, int length2) {
-
-// short circuit equal case
-if (buffer1 == buffer2 &&
-offset1 == offset2 &&
-length1 == length2) {
-return 0;
-}
-
-int end1 = offset1 + length1;
-int end2 = offset2 + length2;
-return Arrays.compareUnsigned(buffer1, offset1, end1, buffer2,
offset2, end2);
-}
+private static class LexicographicByteArrayComparator extends
BytesUtils.LexicographicByteArrayComparator implements ByteArrayComparator {
+// Empty - inherits implementation from BytesUtils, but explicitly
declares it implements the local interface
Review Comment:
sure, thanks Sean.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2869811676
##
build.gradle:
##
@@ -2046,6 +2046,7 @@ project(':clients') {
include "**/org/apache/kafka/common/security/scram/*"
include "**/org/apache/kafka/common/security/token/delegation/*"
include "**/org/apache/kafka/common/serialization/*"
+include "**/org/apache/kafka/common/utils/Bytes.java"
Review Comment:
You are right, it seems a bit odd.
I will move the remaining classes, except Bytes and Time into the internals
package and add the utils package to the javadoc. This would be much cleaner.
Thanks for the input.
By the way, is it "internal" or "internals" because there is an "internals"
package under "org.apache.kafka.streams.state",
"org.apache.kafka.common.config", "org.apache.kafka.common.header",
"org.apache.kafka.common.metrics", and many more.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
brandboat commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2869387390
##
clients/src/main/java/org/apache/kafka/common/utils/internals/BytesUtils.java:
##
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.common.utils.internals;
+
+import org.apache.kafka.common.utils.Bytes;
+
+import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Comparator;
+
+/**
+ * Internal utility class for Bytes-related operations.
+ * This class is for internal Kafka use only and is not part of the public API.
+ */
+public final class BytesUtils {
+
+private BytesUtils() {
+// Utility class, prevent instantiation
+}
+
+/**
+ * Increment the underlying byte array by adding 1. Throws an
IndexOutOfBoundsException if incrementing would cause
+ * the underlying input byte array to overflow.
+ *
+ * @param input - The byte array to increment
+ * @return A new copy of the incremented byte array.
+ */
+public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
+byte[] inputArr = input.get();
+byte[] ret = new byte[inputArr.length];
+int carry = 1;
+for (int i = inputArr.length - 1; i >= 0; i--) {
+if (inputArr[i] == (byte) 0xFF && carry == 1) {
+ret[i] = (byte) 0x00;
+} else {
+ret[i] = (byte) (inputArr[i] + carry);
+carry = 0;
+}
+}
+if (carry == 0) {
+return Bytes.wrap(ret);
+} else {
+throw new IndexOutOfBoundsException();
+}
+}
+
+/**
+ * A byte array comparator based on lexicograpic ordering.
Review Comment:
ditto, `lexicographic`
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +163,36 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
-byte[] inputArr = input.get();
-byte[] ret = new byte[inputArr.length];
-int carry = 1;
-for (int i = inputArr.length - 1; i >= 0; i--) {
-if (inputArr[i] == (byte) 0xFF && carry == 1) {
-ret[i] = (byte) 0x00;
-} else {
-ret[i] = (byte) (inputArr[i] + carry);
-carry = 0;
-}
-}
-if (carry == 0) {
-return wrap(ret);
-} else {
-throw new IndexOutOfBoundsException();
-}
+return BytesUtils.increment(input);
}
/**
* A byte array comparator based on lexicograpic ordering.
Review Comment:
not directly related to this PR, would you mind do a minor typo fix here?
```suggestion
* A byte array comparator based on lexicographic ordering.
```
##
clients/src/main/java/org/apache/kafka/common/utils/internals/BytesUtils.java:
##
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
squah-confluent commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2867723096
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +163,36 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
-byte[] inputArr = input.get();
-byte[] ret = new byte[inputArr.length];
-int carry = 1;
-for (int i = inputArr.length - 1; i >= 0; i--) {
-if (inputArr[i] == (byte) 0xFF && carry == 1) {
-ret[i] = (byte) 0x00;
-} else {
-ret[i] = (byte) (inputArr[i] + carry);
-carry = 0;
-}
-}
-if (carry == 0) {
-return wrap(ret);
-} else {
-throw new IndexOutOfBoundsException();
-}
+return BytesUtils.increment(input);
}
/**
* A byte array comparator based on lexicograpic ordering.
+ * @deprecated This field is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#BYTES_LEXICO_COMPARATOR}
instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
+/**
+ * A byte array comparator interface.
+ *
+ * @deprecated This interface is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils.ByteArrayComparator} instead.
+ */
+@Deprecated(since = "4.3", forRemoval = true)
public interface ByteArrayComparator extends Comparator,
Serializable {
int compare(final byte[] buffer1, int offset1, int length1,
final byte[] buffer2, int offset2, int length2);
}
-private static class LexicographicByteArrayComparator implements
ByteArrayComparator {
-
-@Override
-public int compare(byte[] buffer1, byte[] buffer2) {
-return compare(buffer1, 0, buffer1.length, buffer2, 0,
buffer2.length);
-}
-
-public int compare(final byte[] buffer1, int offset1, int length1,
- final byte[] buffer2, int offset2, int length2) {
-
-// short circuit equal case
-if (buffer1 == buffer2 &&
-offset1 == offset2 &&
-length1 == length2) {
-return 0;
-}
-
-int end1 = offset1 + length1;
-int end2 = offset2 + length2;
-return Arrays.compareUnsigned(buffer1, offset1, end1, buffer2,
offset2, end2);
-}
+private static class LexicographicByteArrayComparator extends
BytesUtils.LexicographicByteArrayComparator implements ByteArrayComparator {
+// Empty - inherits implementation from BytesUtils, but explicitly
declares it implements the local interface
Review Comment:
Thanks for updating the PR!
We can remove this comment. It's not really necessary.
```suggestion
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
chia7712 commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2867533984
##
build.gradle:
##
@@ -2046,6 +2046,7 @@ project(':clients') {
include "**/org/apache/kafka/common/security/scram/*"
include "**/org/apache/kafka/common/security/token/delegation/*"
include "**/org/apache/kafka/common/serialization/*"
+include "**/org/apache/kafka/common/utils/Bytes.java"
Review Comment:
Exposing a single file as public seems a bit odd to me. Maybe we could
follow the pattern from #21412, where we moved the other classes to an internal
package. WDYT?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2866852861
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +161,32 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
-byte[] inputArr = input.get();
-byte[] ret = new byte[inputArr.length];
-int carry = 1;
-for (int i = inputArr.length - 1; i >= 0; i--) {
-if (inputArr[i] == (byte) 0xFF && carry == 1) {
-ret[i] = (byte) 0x00;
-} else {
-ret[i] = (byte) (inputArr[i] + carry);
-carry = 0;
-}
-}
-if (carry == 0) {
-return wrap(ret);
-} else {
-throw new IndexOutOfBoundsException();
-}
+return BytesUtils.increment(input);
}
/**
* A byte array comparator based on lexicograpic ordering.
+ * @deprecated This field is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#BYTES_LEXICO_COMPARATOR}
instead.
*/
-public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
+@Deprecated(since = "4.3", forRemoval = true)
+public static final BytesUtils.ByteArrayComparator BYTES_LEXICO_COMPARATOR
= new LexicographicByteArrayComparator();
-public interface ByteArrayComparator extends Comparator,
Serializable {
-
-int compare(final byte[] buffer1, int offset1, int length1,
-final byte[] buffer2, int offset2, int length2);
+/**
+ * A byte array comparator interface.
+ *
+ * @deprecated This interface is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils.ByteArrayComparator} instead.
+ */
+@Deprecated(since = "4.3", forRemoval = true)
+public interface ByteArrayComparator extends
BytesUtils.ByteArrayComparator {
Review Comment:
True, rolled back
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2866851549
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +161,32 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
-byte[] inputArr = input.get();
-byte[] ret = new byte[inputArr.length];
-int carry = 1;
-for (int i = inputArr.length - 1; i >= 0; i--) {
-if (inputArr[i] == (byte) 0xFF && carry == 1) {
-ret[i] = (byte) 0x00;
-} else {
-ret[i] = (byte) (inputArr[i] + carry);
-carry = 0;
-}
-}
-if (carry == 0) {
-return wrap(ret);
-} else {
-throw new IndexOutOfBoundsException();
-}
+return BytesUtils.increment(input);
}
/**
* A byte array comparator based on lexicograpic ordering.
+ * @deprecated This field is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#BYTES_LEXICO_COMPARATOR}
instead.
*/
-public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
+@Deprecated(since = "4.3", forRemoval = true)
+public static final BytesUtils.ByteArrayComparator BYTES_LEXICO_COMPARATOR
= new LexicographicByteArrayComparator();
-public interface ByteArrayComparator extends Comparator,
Serializable {
-
-int compare(final byte[] buffer1, int offset1, int length1,
-final byte[] buffer2, int offset2, int length2);
+/**
+ * A byte array comparator interface.
+ *
+ * @deprecated This interface is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils.ByteArrayComparator} instead.
+ */
+@Deprecated(since = "4.3", forRemoval = true)
+public interface ByteArrayComparator extends
BytesUtils.ByteArrayComparator {
}
-private static class LexicographicByteArrayComparator implements
ByteArrayComparator {
-
-@Override
-public int compare(byte[] buffer1, byte[] buffer2) {
-return compare(buffer1, 0, buffer1.length, buffer2, 0,
buffer2.length);
-}
-
-public int compare(final byte[] buffer1, int offset1, int length1,
- final byte[] buffer2, int offset2, int length2) {
-
-// short circuit equal case
-if (buffer1 == buffer2 &&
-offset1 == offset2 &&
-length1 == length2) {
-return 0;
-}
-
-int end1 = offset1 + length1;
-int end2 = offset2 + length2;
-return Arrays.compareUnsigned(buffer1, offset1, end1, buffer2,
offset2, end2);
-}
+private static class LexicographicByteArrayComparator extends
BytesUtils.LexicographicByteArrayComparator {
Review Comment:
Thanks again Sean
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r286686
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +161,32 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
-byte[] inputArr = input.get();
-byte[] ret = new byte[inputArr.length];
-int carry = 1;
-for (int i = inputArr.length - 1; i >= 0; i--) {
-if (inputArr[i] == (byte) 0xFF && carry == 1) {
-ret[i] = (byte) 0x00;
-} else {
-ret[i] = (byte) (inputArr[i] + carry);
-carry = 0;
-}
-}
-if (carry == 0) {
-return wrap(ret);
-} else {
-throw new IndexOutOfBoundsException();
-}
+return BytesUtils.increment(input);
}
/**
* A byte array comparator based on lexicograpic ordering.
+ * @deprecated This field is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#BYTES_LEXICO_COMPARATOR}
instead.
*/
-public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
+@Deprecated(since = "4.3", forRemoval = true)
+public static final BytesUtils.ByteArrayComparator BYTES_LEXICO_COMPARATOR
= new LexicographicByteArrayComparator();
Review Comment:
yeah, that would be a breaking change, thanks again :)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] KAFKA-17939: Make Bytes part of the public API (KIP-1247) [kafka]
squah-confluent commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2866822151
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +161,32 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
-byte[] inputArr = input.get();
-byte[] ret = new byte[inputArr.length];
-int carry = 1;
-for (int i = inputArr.length - 1; i >= 0; i--) {
-if (inputArr[i] == (byte) 0xFF && carry == 1) {
-ret[i] = (byte) 0x00;
-} else {
-ret[i] = (byte) (inputArr[i] + carry);
-carry = 0;
-}
-}
-if (carry == 0) {
-return wrap(ret);
-} else {
-throw new IndexOutOfBoundsException();
-}
+return BytesUtils.increment(input);
}
/**
* A byte array comparator based on lexicograpic ordering.
+ * @deprecated This field is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#BYTES_LEXICO_COMPARATOR}
instead.
*/
-public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
+@Deprecated(since = "4.3", forRemoval = true)
+public static final BytesUtils.ByteArrayComparator BYTES_LEXICO_COMPARATOR
= new LexicographicByteArrayComparator();
-public interface ByteArrayComparator extends Comparator,
Serializable {
-
-int compare(final byte[] buffer1, int offset1, int length1,
-final byte[] buffer2, int offset2, int length2);
+/**
+ * A byte array comparator interface.
+ *
+ * @deprecated This interface is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils.ByteArrayComparator} instead.
+ */
+@Deprecated(since = "4.3", forRemoval = true)
+public interface ByteArrayComparator extends
BytesUtils.ByteArrayComparator {
Review Comment:
We can't extend the non public API interface, otherwise it implicitly needs
to be part of the public API.
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +161,32 @@ private static String toString(final byte[] b, int off,
int len) {
*
* @param input - The byte array to increment
* @return A new copy of the incremented byte array.
+ * @deprecated This method is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#increment(Bytes)} instead.
*/
+@Deprecated(since = "4.3", forRemoval = true)
public static Bytes increment(Bytes input) throws
IndexOutOfBoundsException {
-byte[] inputArr = input.get();
-byte[] ret = new byte[inputArr.length];
-int carry = 1;
-for (int i = inputArr.length - 1; i >= 0; i--) {
-if (inputArr[i] == (byte) 0xFF && carry == 1) {
-ret[i] = (byte) 0x00;
-} else {
-ret[i] = (byte) (inputArr[i] + carry);
-carry = 0;
-}
-}
-if (carry == 0) {
-return wrap(ret);
-} else {
-throw new IndexOutOfBoundsException();
-}
+return BytesUtils.increment(input);
}
/**
* A byte array comparator based on lexicograpic ordering.
+ * @deprecated This field is not part of the public API and will be
removed in version 5.0.
+ * Internal Kafka code should use {@link
org.apache.kafka.common.utils.internals.BytesUtils#BYTES_LEXICO_COMPARATOR}
instead.
*/
-public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
+@Deprecated(since = "4.3", forRemoval = true)
+public static final BytesUtils.ByteArrayComparator BYTES_LEXICO_COMPARATOR
= new LexicographicByteArrayComparator();
Review Comment:
We can't change the type of this field unfortunately.
```suggestion
public static final ByteArrayComparator BYTES_LEXICO_COMPARATOR = new
LexicographicByteArrayComparator();
```
##
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##
@@ -146,57 +161,32
