YongGoose commented on code in PR #7311:
URL: https://github.com/apache/incubator-seata/pull/7311#discussion_r2063783841
##########
compressor/seata-compressor-gzip/src/main/java/org/apache/seata/compressor/gzip/GzipUtil.java:
##########
@@ -64,4 +64,74 @@ public static byte[] decompress(byte[] bytes) {
}
}
+ /**
+ * Compress the data if it exceeds the threshold and add a flag byte to
indicate compression status.
+ * This method adds a single byte at the beginning of the data to indicate
whether it is compressed.
+ *
+ * @param bytes the bytes to potentially compress
+ * @param threshold the threshold in bytes. If the data size is below
this, no compression is performed
+ * @return byte array with compression flag + original/compressed data
+ */
+ public static byte[] compressWithFlag(byte[] bytes, int threshold) {
+ if (bytes == null) {
+ throw new NullPointerException("bytes is null");
+ }
+
+ // Don't compress small data
+ if (bytes.length < threshold) {
+ return addCompressionFlag(bytes, false);
+ }
+
+ byte[] compressed = compress(bytes);
+
+ if (compressed.length < bytes.length) {
+ return addCompressionFlag(compressed, true);
+ } else {
+ return addCompressionFlag(bytes, false);
+ }
+ }
+
+ /**
+ * Decompress the data if the flag indicates it is compressed.
+ * This method expects the first byte to be a compression flag.
+ *
+ * @param bytes the bytes with compression flag
+ * @return the decompressed data
+ */
+ public static byte[] decompressWithFlag(byte[] bytes) {
+ if (bytes == null || bytes.length == 0) {
+ throw new NullPointerException("bytes is null or empty");
+ }
+
+ boolean isCompressed = bytes[0] == COMPRESSED_FLAG;
+ byte[] data = new byte[bytes.length - 1];
+ System.arraycopy(bytes, 1, data, 0, data.length);
+
+ return isCompressed ? decompress(data) : data;
+ }
+
+ /**
+ * Add a compression flag byte to the beginning of the data.
+ *
+ * @param bytes the original data
+ * @param isCompressed whether the data is compressed
+ * @return byte array with flag + data
+ */
+ private static byte[] addCompressionFlag(byte[] bytes, boolean
isCompressed) {
+ byte[] result = new byte[bytes.length + 1];
+ result[0] = isCompressed ? COMPRESSED_FLAG : UNCOMPRESSED_FLAG;
+ System.arraycopy(bytes, 0, result, 1, bytes.length);
+ return result;
+ }
+
+ /**
+ * Check if the data has the compression flag and is marked as compressed.
+ * This is the preferred method to check compression status when using the
new format.
+ *
+ * @param bytes the bytes to check
+ * @return true if the first byte indicates the data is compressed
+ */
+ public static boolean hasCompressionFlag(byte[] bytes) {
+ return bytes != null && bytes.length > 0 && bytes[0] ==
COMPRESSED_FLAG;
Review Comment:
I think that's a good idea, and I agree with you.
Considering users who may continue using existing data files, I modified the
logic in `BranchSession` to first check the header against
`GZIPInputStream.GZIP_MAGIC` to determine whether compression is enabled. If
that doesn't match, it then checks against the newly added `COMPRESSED_FLAG`.
```java
private String decodeLockKey(byte[] lockKeyBytes) {
if (GzipUtil.isCompressData(lockKeyBytes)) {
try {
return new String(GzipUtil.uncompress(lockKeyBytes));
} catch (IOException e) {
throw new RuntimeException("decompress lockKey error", e);
}
}
if (GzipUtil.hasCompressionFlag(lockKeyBytes)) {
return new String(GzipUtil.decompressWithFlag(lockKeyBytes));
}
return new String(lockKeyBytes);
}
```
However, there's still a potential issue here. If an existing data file
wasn't compressed, but its header coincidentally matches the new
`COMPRESSED_FLAG`, the system could mistakenly identify the file as compressed.
> I did increase the size of the COMPRESSED_FLAG to `2 bytes`, considering
this case.
I believe this could lead to problems—what are your thoughts on this?
--
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: notifications-unsubscr...@seata.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@seata.apache.org
For additional commands, e-mail: notifications-h...@seata.apache.org
