support compression for IPC #1855

liukun4515 · 2022-06-13T03:13:47Z

Which issue does this PR close?

Closes #1709
Closes #70

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

arrow/src/ipc/reader.rs

arrow/src/ipc/writer.rs

…sion

liukun4515 · 2022-06-18T16:46:11Z

arrow/src/ipc/writer.rs

+    };
+    let len = data.len() as i64;
+    // TODO: don't need to pad each buffer, and just need to pad the tail of the message body
+    // let pad_len = pad_to_8(len as u32);


I read format/IPC spec and got that we don't need to align each buffer and pad the 0 to the tail of each buffer, just pad the 0 to the tail of message body.
@martin-g @alamb
Is there any error for my understanding for the IPC format?
Please feel free to point out.

If this make sense to you, I will drop these commented code.

codecov-commenter · 2022-06-18T17:20:00Z

Codecov Report

Merging #1855 (7c6c2ee) into master (c7f89e1) will increase coverage by 0.04%.
The diff coverage is 85.47%.

@@            Coverage Diff             @@
##           master    #1855      +/-   ##
==========================================
+ Coverage   83.42%   83.47%   +0.04%     
==========================================
  Files         214      215       +1     
  Lines       57015    57286     +271     
==========================================
+ Hits        47567    47819     +252     
- Misses       9448     9467      +19

Impacted Files	Coverage Δ
arrow/src/ipc/compression/ipc_compression.rs	`76.36% <76.36%> (ø)`
arrow/src/ipc/writer.rs	`82.19% <84.45%> (+0.40%)`	⬆️
arrow/src/ipc/reader.rs	`91.16% <98.18%> (+0.33%)`	⬆️
parquet_derive/src/parquet_field.rs	`65.53% <0.00%> (-0.69%)`	⬇️
parquet/src/util/hash_util.rs	`95.16% <0.00%> (-0.50%)`	⬇️
arrow/src/array/array_dictionary.rs	`91.53% <0.00%> (-0.39%)`	⬇️
parquet/src/util/bit_util.rs	`92.68% <0.00%> (-0.33%)`	⬇️
arrow/src/record_batch.rs	`93.89% <0.00%> (-0.11%)`	⬇️
parquet/src/encodings/encoding.rs	`93.62% <0.00%> (-0.03%)`	⬇️
arrow/src/json/reader.rs	`84.58% <0.00%> (ø)`
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c7f89e1...7c6c2ee. Read the comment docs.

liukun4515 · 2022-06-19T02:04:44Z

hi @andygrove @alamb , I have implemented the IPC compression, but the ci on AMD 32 fails.
I have no idea to fix it, can you help me?

liukun4515 · 2022-06-19T03:02:51Z

After this pr merged, I will enable the IT test for 2.0.0-compression

liukun4515 · 2022-06-19T08:00:52Z

The integration test was failed from https://github.com/apache/arrow-rs/runs/6952125461?check_suite_focus=true#step:6:15070
and check the cause of c++ exception: https://github.com/apache/arrow/blob/4ade394a7a0fa22ecb8ef2e3b4dc8bb42e599274/cpp/src/arrow/ipc/reader.cc#L198

I'm confused about why we need to make 8 bytes aligned for each buffer

liukun4515 · 2022-06-19T09:14:39Z

hi @andygrove @alamb , I have implemented the IPC compression, but the ci on AMD 32 fails. I have no idea to fix it, can you help me?

@martin-g Could you please take a look this ci problem?

liukun4515 · 2022-06-20T02:23:12Z

hi @andygrove @alamb , I have implemented the IPC compression, but the ci on AMD 32 fails. I have no idea to fix it, can you help me?

@martin-g Could you please take a look this ci problem?

I think the issue has been resolved

alamb · 2022-06-21T19:55:02Z

Thanks @liukun4515 -- I will try and find time tomorrow to review this PR

viirya · 2022-06-21T21:18:28Z

arrow/Cargo.toml

@@ -60,7 +63,7 @@ multiversion = { version = "0.6.1", default-features = false }
 bitflags = { version = "1.2.1", default-features = false }

 [features]
-default = ["csv", "ipc", "test_utils"]
+default = ["csv", "ipc", "test_utils", "zstd", "lz4"]


So by default we will include zstd, lz4 dependencies? If we don't use ipc or don't use compressed ipc, seems the dependencies are not necessary?

I agree -- I suggest something like

Suggested change

default = ["csv", "ipc", "test_utils", "zstd", "lz4"]

default = ["csv", "ipc", "test_utils"]

Yes, include the dependency by default.
buffer compression protocol is common protocol for all languages in the arrow ecosystem, if we use the compression/decompression as optional, we can read the file or stream from compressed side.
2.0.0 compression has been implemented in the Java, C++ and other languages, If your rust server receiver a message compressed by the protocol, we can read them by default.

I'd be in favour of supporting adding the deps toipc but not creating ipc_compression separately. The inconvenience of a larger binary is a lesser problem than the runtime error of not supporting compression by omission.
The latter would need a rebuild.

I have encountered an issue on cross-compiling lz4 crate on some platform. We don't use ipc so we can choose not to include ipc feature at all. I'm wondering if ipc compression can't be excluded, there might be some issues like that. Although this sounds like corner case.

I have add the feature ipc_compression = "lz4,zstd" as an option, if you want to compile with the ipc compression feature.

arrow/src/ipc/reader.rs

viirya · 2022-06-21T21:27:15Z

arrow/src/ipc/reader.rs

+                // TODO consider the error result
+                #[cfg(any(feature = "zstd,lz4", test))]


Hmm, if these features are not used, what will it happened? I think an explicit error is necessary.

agree with you, @viirya . I have no idea and how to control this.
Could you please give me some suggestion or advice?

I follow the #[cfg(any(feature usage like other model to resolve the error from the CI

cargo build --features=csv,ipc,simd,lz4,zstd --target wasm32-unknown-unknown

If I don't use this method, I can't pass the ci for target wasm32-unknown-unknown
I search the reason and find the issue #180 and #180 which have the same problem.
@alamb

viirya · 2022-06-21T22:05:43Z

arrow/src/ipc/writer.rs

+                    })
+                }
+            }
+            z => panic!("Unsupported ipc::MetadataVersion {:?}", z),


ipc::MetadataVersion::V4 seems also hitting this error? But actually there is CompressionCodecType::NoCompression indicating no compression is used.

Also given that this function returns a Result it seems like we could return a proper error here rather than panicing

I just copy some code from try_new.
try_new just use the CompressionCodecType::NoCompression and it is same with older version.
try_new_with_compression want to open the compression, but I think I made a mistake.
I should use the CompressionType instead of CompressionCodecType and make sure the compression is enable

viirya · 2022-06-21T22:11:30Z

arrow/src/ipc/writer.rs

+                #[cfg(any(feature = "zstd,lz4", test))]
+                compression_codec
+                    .compress(buffer.as_slice(), &mut _compression_buffer)
+                    .unwrap();


I think we can prevent using compression option in try_new_with_compression if these features are not included.

@viirya comments like above reply
Maybe I can resolve this option or compile issue in the next issue or pull request.
In this pull request, we can focus on the protocal of IPC compression

viirya · 2022-06-21T22:12:18Z

arrow/src/ipc/writer.rs

        // write the schema, set the written bytes to the schema + header
        let encoded_message = data_gen.schema_to_bytes(schema, &write_options);
        let (meta, data) = write_message(&mut writer, encoded_message, &write_options)?;
        Ok(Self {
            writer,
            write_options,
            schema: schema.clone(),
-            block_offsets: meta + data + 8,
+            block_offsets: meta + data + header_size,


What is this for?

try_new_with_options is used to write schema to the file or stream.
In the arrow format of IPC format, the layout is from the doc https://arrow.apache.org/docs/format/Columnar.html#ipc-file-format

First, we will write
<magic number "ARROW1">
<empty padding bytes [to 8 byte boundary]>
the size of above part is 8bytes and is the length of header_size

alamb

Thanks @liukun4515 -- I think this is looking pretty good. The biggest thing I would like to sort out the feature flags and keeping the compression code a bit more isolated. Otherwise this is looking close

alamb · 2022-06-22T12:45:48Z

arrow/Cargo.toml

@@ -60,7 +63,7 @@ multiversion = { version = "0.6.1", default-features = false }
 bitflags = { version = "1.2.1", default-features = false }

 [features]
-default = ["csv", "ipc", "test_utils"]
+default = ["csv", "ipc", "test_utils", "zstd", "lz4"]


I agree -- I suggest something like

Suggested change

default = ["csv", "ipc", "test_utils", "zstd", "lz4"]

default = ["csv", "ipc", "test_utils"]

arrow/Cargo.toml

alamb · 2022-06-22T12:51:09Z

arrow/src/ipc/compression/ipc_compression.rs

+            match self {
+                CompressionCodecType::Lz4Frame => {
+                    let mut encoder = lz4::EncoderBuilder::new().build(output).unwrap();
+                    encoder.write_all(input).unwrap();


I suggest passing the errors back out of here directly (as compress returns a Result) rather than unwrap() which will panic on error

alamb · 2022-06-22T12:52:39Z

arrow/src/ipc/compression/ipc_compression.rs

+    }
+}
+
+#[cfg(any(feature = "zstd,lz4", test))]


I wonder if you can put this guard on the entire mod ipc_compression statement so that the entire module (including the test) is not compiled unless that feature is active

arrow/src/ipc/mod.rs

alamb · 2022-06-22T13:00:06Z

arrow/src/ipc/writer.rs

+            _ => {
+                if metadata_version != ipc::MetadataVersion::V5 {
+                    return Err(ArrowError::InvalidArgumentError(
+                        "Compress buffer just support from metadata v5".to_string(),


Suggested change

"Compress buffer just support from metadata v5".to_string(),

"Compression only supported in metadata v5 and above".to_string(),

alamb · 2022-06-22T13:00:46Z

arrow/src/ipc/writer.rs

+        match batch_compression_type {
+            CompressionCodecType::NoCompression => {}
+            _ => {
+                if metadata_version != ipc::MetadataVersion::V5 {


should this check be < instead of != to cover future versions?

Suggested change

if metadata_version != ipc::MetadataVersion::V5 {

if metadata_version < ipc::MetadataVersion::V5 {

alamb · 2022-06-22T13:01:42Z

arrow/src/ipc/writer.rs

+                    })
+                }
+            }
+            z => panic!("Unsupported ipc::MetadataVersion {:?}", z),


Also given that this function returns a Result it seems like we could return a proper error here rather than panicing

alamb · 2022-06-22T13:07:07Z

arrow/src/ipc/writer.rs

 }

 impl IpcWriteOptions {
+    pub fn try_new_with_compression(


I wonder if you could avoid the duplication here with more of a Builder style:

impl IpcWriteOptions { pub fn with_compression(mut self, batch_compression_type: CompressionCodecType) -> Result<Self> { .. // do checks here self.batch_compresson_type = batch_compression_type; Ok(self) } ... }

Then one could use it like:

let options = IpcWriteOptions::try_new(8, false, ipc::MetadataVersion::v5)? .with_compression(CompressionCodecType::LZ4)?; ...

The constructor may be refactor by follow up PR

alamb · 2022-06-22T13:08:01Z

arrow/src/ipc/writer.rs

+            (buffer.as_slice(), origin_buffer_len as i64)
+        }
+        CompressionCodecType::Lz4Frame | CompressionCodecType::Zstd => {
+            if (origin_buffer_len as i64) == LENGTH_EMPTY_COMPRESSED_DATA {


It seems like this code should throw an "Not supported" error (or panic if it is encountered without support compiled in)

alamb · 2022-06-24T14:13:50Z

What is the status of this PR? Is it ready to go? Do we need to mess with the feature flags more?

viirya · 2022-06-24T16:15:39Z

I think there are some comments are not addressed yet?

liukun4515 · 2022-06-25T14:00:48Z

Hi @alamb @viirya
I will continue to do this work next week after I learn the feature flag and other something.
I will make the pr as a draft.
After the pr is ready and address all the comments, I will reopen it.
Sorry for that.

liukun4515 · 2022-08-01T03:02:13Z

arrow/src/ipc/reader.rs

+            }
+        }
+        CompressionCodecType::NoCompression => Buffer::from(buf_data),
+        _ => {


How to handle this?
If the rust service compiled without the ipc_compression and receive a message with the ipc compression feature.

I think returning an error is the correct way but as you have identifed above you can't do that without changing the signature to Result<Buffer> -- but since decompression can fail we probably need to make the change

@alamb
I file a new issue to track this, and will submit a sub pr for this.

liukun4515 · 2022-08-01T03:03:37Z

arrow/src/ipc/reader.rs

+    }
+    match compression_codec {
+        CompressionCodecType::Lz4Frame | CompressionCodecType::Zstd
+            if cfg!(feature = "ipc_compression") || cfg!(test) =>


need to check the compile options, if receive a message or ipc message with the compression feature.

viirya · 2022-08-01T03:09:32Z

arrow/src/ipc/compression/ipc_compression.rs

+                    Ok(())
+                }
+                CompressionCodecType::Zstd => {
+                    let mut encoder = zstd::Encoder::new(output, 0).unwrap();


Should we pass the error out?

fix, and pass the error out.

viirya · 2022-08-01T03:11:16Z

arrow/src/ipc/compression/ipc_compression.rs

+                        Err(e) => Err(e.into()),
+                    }
+                }
+                _ => Ok(input.len()),


Is this for CompressionCodecType::NoCompression? If so, do we need to copy form input to output?

remove the NoCompression in the CompressionCodecType.

just handle the LZ4 and ZSTD branch.

viirya · 2022-08-01T03:15:19Z

arrow/src/ipc/reader.rs

+                Buffer::from(data)
+            } else {
+                // decompress data using the codec
+                let mut _uncompressed_buffer = Vec::new();


We know decompressed_length at this point. Should we allocate the vector with enough capacity?

good point, we can init the vec with the capacity.

viirya · 2022-08-01T03:16:24Z

arrow/src/ipc/reader.rs

+///   -1: indicate that the data that follows is not compressed
+///    0: indicate that there is no data
+///   positive number: indicate the uncompressed length for the following data
+fn read_uncompressed_size(buffer: &[u8]) -> i64 {


#[inline]?

viirya · 2022-08-01T03:17:24Z

arrow/src/ipc/reader.rs

+        Some(compression) => match compression.codec() {
+            CompressionType::ZSTD => CompressionCodecType::Zstd,
+            CompressionType::LZ4_FRAME => CompressionCodecType::Lz4Frame,
+            _ => CompressionCodecType::NoCompression,


Is there other option?

No, in current version, just support the LZ4 and zstd.

liukun4515 · 2022-08-01T03:18:38Z

arrow/src/ipc/reader.rs

+/// uncompressed length may be set to -1 to indicate that the data that
+/// follows is not compressed, which can be useful for cases where
+/// compression does not yield appreciable savings.
+fn read_buffer(


TODO:
we should change the output arg to Result<Buffer> and return error message if the buffer can't be read.

do you still intend to make this change? Or is it planned for a subsequent PR?

arrow/src/ipc/writer.rs

viirya · 2022-08-01T03:38:01Z

arrow/src/ipc/writer.rs

+        }
+        CompressionCodecType::Lz4Frame | CompressionCodecType::Zstd => {
+            if (origin_buffer_len as i64) == LENGTH_EMPTY_COMPRESSED_DATA {
+                (buffer, 0)


Suggested change

(buffer, 0)

(buffer, LENGTH_EMPTY_COMPRESSED_DATA)

viirya · 2022-08-01T03:39:33Z

arrow/src/ipc/writer.rs

+    // padding and make offset 8 bytes aligned
+    let pad_len = pad_to_8(len as u32) as i64;


Is it padding to data len? Or total len?

In each buffer, we have two struct, one is the metadata which store offset and actual len of data, the other is the data.
The actual len is the total_len.
The pad_len is just used to align the buffer.

liukun4515 · 2022-08-03T07:10:36Z

@viirya @alamb PTAL

alamb · 2022-08-03T21:50:33Z

arrow/Cargo.toml

+lz4 = { version = "1.23", default-features = false }
+zstd = { version = "0.11", default-features = false }


I don't think these need to be in dev dependencies do they? If they are already in the dependencies of the crate?

we can’t remove the lz4 and zstd from dev-dependency.
The ipc_compression is not in the default feature, we can‘t run cargo test with the lz4 and zstd lib.
But we need to run the ipc compiression UT in CI

I try to remove these from the dev-dependency, can run cargo test.
I got the compile error.

error[E0433]: failed to resolve: use of undeclared crate or module `lz4` --> arrow/src/ipc/compression/ipc_compression.rs:57:39 | 57 | let mut encoder = lz4::EncoderBuilder::new().build(output)?; | ^^^ use of undeclared crate or module `lz4` error[E0433]: failed to resolve: use of undeclared crate or module `zstd` --> arrow/src/ipc/compression/ipc_compression.rs:65:39 | 65 | let mut encoder = zstd::Encoder::new(output, 0)?; | ^^^^ use of undeclared crate or module `zstd` | help: there is a crate or module with a similar name | 65 | let mut encoder = std::Encoder::new(output, 0)?; | ~~~

@alamb

alamb · 2022-08-03T21:51:34Z

arrow/src/ipc/reader.rs

+/// uncompressed length may be set to -1 to indicate that the data that
+/// follows is not compressed, which can be useful for cases where
+/// compression does not yield appreciable savings.
+fn read_buffer(


do you still intend to make this change? Or is it planned for a subsequent PR?

alamb · 2022-08-03T21:53:29Z

arrow/src/ipc/reader.rs

+            }
+        }
+        CompressionCodecType::NoCompression => Buffer::from(buf_data),
+        _ => {


I think returning an error is the correct way but as you have identifed above you can't do that without changing the signature to Result<Buffer> -- but since decompression can fail we probably need to make the change

alamb · 2022-08-03T21:55:26Z

arrow/src/ipc/reader.rs

+                // decompress data using the codec
+                let mut _uncompressed_buffer =
+                    Vec::with_capacity(decompressed_length as usize);
+                let _input_data = &buf_data[(LENGTH_OF_PREFIX_DATA as usize)..];


Something still isn't quite right with this code -- instead of gating the code on the test feature, I think the more typical pattern is to gate the entire test on the ipc_compression feature

So something like

#[cfg(ipc_compression)] #[test] fn read_generated_streams_200() { let testdata = crate::util::test_util::arrow_test_data(); let version = "2.0.0-compression"; ... }

#[cfg(any(feature = "ipc_compression", test))] this is not the feature of test, just feature ipc_compression or test.
It only can be compiled with the compile flag feature="ipc_compression" or test.

There are some usage like in parquet.

arrow-rs/parquet/src/compression.rs

Line 76 in 30c94db

#[cfg(any(feature = "lz4", test))]

alamb · 2022-08-06T10:29:08Z

@liukun4515 -- perhaps I can find some time to try and help with this PR. I will try to do so tomorrow

liukun4515 · 2022-08-06T11:59:51Z

@liukun4515 -- perhaps I can find some time to try and help with this PR. I will try to do so tomorrow

Thank you, you can also ping me through the slack.

alamb · 2022-08-06T13:44:34Z

Thank you, you can also ping me through the slack.

Thanks @liukun4515 -- I think I have gotten to the point where I can't offer any more specific suggestions on structure without trying it myself. I hope to try and rearrange the feature-flags and make a proposed PR to your branch.

I won't have a chance to do it until tomorrow however.

alamb · 2022-08-07T11:46:49Z

@liukun4515 -- I made good progress on sorting out the feature flags. A draft PR is here liukun4515#1 in case you want to see the direction I am headed.

I sadly ran out of time today, but I will try and finish it up tomorrow.

liukun4515 · 2022-08-08T14:51:24Z

@liukun4515 -- I made good progress on sorting out the feature flags. A draft PR is here liukun4515#1 in case you want to see the direction I am headed.

I sadly ran out of time today, but I will try and finish it up tomorrow.

thanks for your help!!!
I will review your work tomorrow, because I am busy doing and discussing about optimization of decimal.

alamb · 2022-08-08T15:22:22Z

I will review your work tomorrow, because I am busy doing and discussing about optimization of decimal.

Thank you -- I hope to push up an update shortly. It turned into a larger change than I was expecting

alamb · 2022-08-08T16:13:44Z

@liukun4515

Here is my proposal (the PR to liukun4515#1 is now rendered terribly): #2369

liukun4515 · 2022-08-11T02:19:03Z

Thanks for @alamb cooperation.
I will close this pr.

github-actions bot added the arrow Changes to the arrow crate label Jun 13, 2022

liukun4515 force-pushed the flight_data_compression branch from e79ecf8 to 7dbd4d6 Compare June 15, 2022 10:30

martin-g reviewed Jun 15, 2022

View reviewed changes

liukun4515 mentioned this pull request Jun 17, 2022

ARROW-11417: Add integration files for buffer compression apache/arrow-testing#56

Merged

liukun4515 added 2 commits June 18, 2022 21:34

support compression for IPC

e51eca3

Merge remote-tracking branch 'origin/master' into flight_data_compres…

ede5115

…sion

liukun4515 force-pushed the flight_data_compression branch from 7dbd4d6 to ede5115 Compare June 18, 2022 13:52

fix conflict

67e7de5

liukun4515 marked this pull request as ready for review June 18, 2022 16:37

liukun4515 commented Jun 18, 2022

View reviewed changes

edit toml

58488a3

liukun4515 added 3 commits June 19, 2022 09:52

fix clippy and format

932e381

format doc

fcc4f5f

format code

5b0d711

liukun4515 mentioned this pull request Jun 19, 2022

enable the IT test for 2.0.0-compression #1907

Closed

add padding for tail of each buffer

0c61067

liukun4515 added 2 commits June 19, 2022 22:21

try fix the arrow lz4 and zstd

9ac4b01

add lz4,zstd for compression cfg

49edfc8

viirya reviewed Jun 21, 2022

View reviewed changes

alamb reviewed Jun 22, 2022

View reviewed changes

alamb requested a review from viirya July 31, 2022 11:12

liukun4515 requested review from alamb and nevi-me August 1, 2022 02:48

liukun4515 commented Aug 1, 2022

View reviewed changes

viirya reviewed Aug 1, 2022

View reviewed changes

liukun4515 commented Aug 1, 2022

View reviewed changes

arrow/src/ipc/writer.rs Show resolved Hide resolved

viirya reviewed Aug 1, 2022

View reviewed changes

address comments

7f90fb2

liukun4515 requested a review from viirya August 1, 2022 14:43

alamb reviewed Aug 3, 2022

View reviewed changes

liukun4515 mentioned this pull request Aug 6, 2022

Change the output of read_buffer Arrow IPC API to return Result<_> #2342

Closed

alamb mentioned this pull request Aug 7, 2022

Rework ipc_compression feature flags liukun4515/arrow-rs#1

Draft

6 tasks

alamb mentioned this pull request Aug 8, 2022

support compression for IPC with revamped feature flags #2369

Merged

liukun4515 closed this Aug 11, 2022

	default = ["csv", "ipc", "test_utils", "zstd", "lz4"]
	default = ["csv", "ipc", "test_utils"]

		// TODO consider the error result
		#[cfg(any(feature = "zstd,lz4", test))]

	"Compress buffer just support from metadata v5".to_string(),
	"Compression only supported in metadata v5 and above".to_string(),

	if metadata_version != ipc::MetadataVersion::V5 {
	if metadata_version < ipc::MetadataVersion::V5 {

		// padding and make offset 8 bytes aligned
		let pad_len = pad_to_8(len as u32) as i64;

		lz4 = { version = "1.23", default-features = false }
		zstd = { version = "0.11", default-features = false }

support compression for IPC #1855

support compression for IPC #1855

Conversation

liukun4515 commented Jun 13, 2022 • edited by alamb

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 18, 2022 • edited

Codecov Report

liukun4515 commented Jun 19, 2022

liukun4515 commented Jun 19, 2022

liukun4515 commented Jun 19, 2022

liukun4515 commented Jun 19, 2022

liukun4515 commented Jun 20, 2022

alamb commented Jun 21, 2022

viirya Jun 21, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nevi-me Jun 23, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alamb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alamb commented Jun 24, 2022

viirya commented Jun 24, 2022

liukun4515 commented Jun 25, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liukun4515 Aug 1, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liukun4515 Aug 1, 2022 • edited

Choose a reason for hiding this comment

liukun4515 commented Aug 3, 2022

Choose a reason for hiding this comment

liukun4515 Aug 6, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liukun4515 commented Jun 13, 2022 •

edited by alamb

codecov-commenter commented Jun 18, 2022 •

edited

viirya Jun 21, 2022 •

edited

nevi-me Jun 23, 2022 •

edited

liukun4515 Aug 1, 2022 •

edited

liukun4515 Aug 1, 2022 •

edited

liukun4515 Aug 6, 2022 •

edited

liukun4515 Aug 6, 2022 •

edited