Copilot commented on code in PR #1700:
URL: https://github.com/apache/fluss/pull/1700#discussion_r2351710772
##########
fluss-server/src/main/java/org/apache/fluss/server/zk/data/LakeTableSnapshot.java:
##########
@@ -32,10 +32,11 @@ public class LakeTableSnapshot {
// the log offset of the bucket
- // mapping from bucket id to log start/end offset,
+ // mapping from bucket id to log start/end offset or max timestamp,
// will be null if log offset is unknown such as reading the snapshot of
primary key table
Review Comment:
The comment should be more precise. It describes mappings to 'log start/end
offset or max timestamp' but the actual fields are separate maps for start
offsets, end offsets, and max timestamps.
```suggestion
// Mappings from bucket id to log start offset, log end offset, and max
timestamp, respectively.
// Each map will be null if the corresponding value is unknown, such as
when reading the snapshot of a primary key table.
```
##########
fluss-flink/fluss-flink-common/src/main/java/org/apache/fluss/flink/tiering/source/TableBucketWriteResult.java:
##########
@@ -42,12 +42,16 @@ public class TableBucketWriteResult<WriteResult> implements
Serializable {
// null when the bucket is not for a partition
@Nullable private final String partitionName;
- // will be null when no any data write, such as for tiering a empty log
split
+ // will be null when no any data write, such as for tiering an empty log
split
Review Comment:
Change 'no any data write' to 'no data is written' for better grammar.
```suggestion
// will be null when no data is written, such as for tiering an empty
log split
```
##########
fluss-flink/fluss-flink-common/src/test/java/org/apache/fluss/flink/tiering/committer/TieringCommitOperatorTest.java:
##########
@@ -413,13 +483,15 @@ private void verifyLakeSnapshot(
TablePath tablePath,
long tableId,
long expectedSnapshotId,
- Map<TableBucket, Long> expectedLogEndOffsets)
+ Map<TableBucket, Long> expectedLogEndOffsets,
+ Map<TableBucket, Long> expectedMaxTimestamp)
throws Exception {
LakeSnapshot lakeSnapshot =
admin.getLatestLakeSnapshot(tablePath).get();
assertThat(lakeSnapshot.getSnapshotId()).isEqualTo(expectedSnapshotId);
assertThat(lakeSnapshot.getTableBucketsOffset()).isEqualTo(expectedLogEndOffsets);
+
assertThat(lakeSnapshot.getTableBucketsTimestamp()).isEqualTo(expectedMaxTimestamp);
- // check the tableId has been send to mark finished
+ // check the tableId has been sent to mark finished
Review Comment:
Good correction of 'send' to 'sent' for proper past tense.
##########
fluss-flink/fluss-flink-common/src/test/java/org/apache/fluss/flink/tiering/committer/TieringCommitOperatorTest.java:
##########
@@ -433,15 +505,17 @@ private void verifyLakeSnapshot(
long tableId,
long expectedSnapshotId,
Map<TableBucket, Long> expectedLogEndOffsets,
+ Map<TableBucket, Long> expectedMaxTimestamp,
Map<Long, String> expectedPartitionIdByName,
String failedReason)
throws Exception {
LakeSnapshot lakeSnapshot =
admin.getLatestLakeSnapshot(tablePath).get();
assertThat(lakeSnapshot.getSnapshotId()).isEqualTo(expectedSnapshotId);
assertThat(lakeSnapshot.getTableBucketsOffset()).isEqualTo(expectedLogEndOffsets);
+
assertThat(lakeSnapshot.getTableBucketsTimestamp()).isEqualTo(expectedMaxTimestamp);
assertThat(lakeSnapshot.getPartitionNameById()).isEqualTo(expectedPartitionIdByName);
- // check the tableId has been send to mark failed
+ // check the tableId has been sent to mark failed
Review Comment:
Good correction of 'send' to 'sent' for proper past tense.
```suggestion
// check the tableId has been sent to mark as failed
```
--
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]
