You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/10/21 08:09:27 UTC
[GitHub] [flink-table-store] tsreaper opened a new pull request, #328: [FLINK-29714] Merge TableWrite and TableCompact into one interface
tsreaper opened a new pull request, #328:
URL: https://github.com/apache/flink-table-store/pull/328
To make sure that full compaction is triggered constantly for every written bucket regardless of failure, we need to add `compact` interface to `TableWrite` so that Flink sink operators can trigger compaction when needed.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] JingsongLi merged pull request #328: [FLINK-29714] Merge TableWrite and TableCompact into one interface
Posted by GitBox <gi...@apache.org>.
JingsongLi merged PR #328:
URL: https://github.com/apache/flink-table-store/pull/328
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] JingsongLi commented on a diff in pull request #328: [FLINK-29714] Merge TableWrite and TableCompact into one interface
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on code in PR #328:
URL: https://github.com/apache/flink-table-store/pull/328#discussion_r1002843817
##########
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/mergetree/compact/CompactStrategy.java:
##########
@@ -37,4 +37,7 @@ public interface CompactStrategy {
* </ul>
*/
Optional<CompactUnit> pick(int numLevels, List<LevelSortedRun> runs);
+
+ /** Pick compaction unit from runs. However this compaction is forced by the user. */
+ Optional<CompactUnit> forcedPick(int numLevels, List<LevelSortedRun> runs);
Review Comment:
Do we need to add this method? It looks like a fixed implementation.
##########
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/mergetree/compact/MergeTreeCompactManager.java:
##########
@@ -80,43 +77,52 @@ public void addNewFile(DataFileMeta file) {
}
@Override
- public void triggerCompaction() {
- if (taskFuture != null) {
- return;
+ public void triggerCompaction(boolean forcedCompaction) {
+ Optional<CompactUnit> optionalUnit;
+ if (forcedCompaction) {
Review Comment:
I think `full compaction` is better than force compaction.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] JingsongLi commented on a diff in pull request #328: [FLINK-29714] Merge TableWrite and TableCompact into one interface
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on code in PR #328:
URL: https://github.com/apache/flink-table-store/pull/328#discussion_r1004006577
##########
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/mergetree/MergeTreeWriter.java:
##########
@@ -137,6 +137,11 @@ public void write(KeyValue kv) throws Exception {
}
}
+ @Override
+ public void compact() throws Exception {
Review Comment:
Can we just use `fullCompact`?
##########
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/mergetree/MergeTreeWriter.java:
##########
@@ -252,9 +257,9 @@ private void updateCompactResult(CompactResult result) {
compactAfter.addAll(result.after());
}
- private void submitCompaction() throws Exception {
- trySyncLatestCompaction(false);
- compactManager.triggerCompaction();
+ private void submitCompaction(boolean forcedCompaction) throws Exception {
Review Comment:
`forcedCompaction` -> `forcedFullCompaction`?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] JingsongLi commented on a diff in pull request #328: [FLINK-29714] Merge TableWrite and TableCompact into one interface
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on code in PR #328:
URL: https://github.com/apache/flink-table-store/pull/328#discussion_r1002982641
##########
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/compact/LazyCompactManager.java:
##########
@@ -36,13 +38,25 @@ public boolean shouldWaitCompaction() {
}
@Override
- public void addNewFile(DataFileMeta file) {}
+ public void addNewFile(DataFileMeta file) {
+ wrapped.addNewFile(file);
+ }
+
+ @Override
+ public void triggerCompaction(boolean fullCompaction) {
+ if (fullCompaction) {
+ wrapped.triggerCompaction(true);
Review Comment:
We can throw unsupported exception 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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org