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 2020/10/20 07:43:57 UTC
[GitHub] [flink] JingsongLi opened a new pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
JingsongLi opened a new pull request #13697:
URL: https://github.com/apache/flink/pull/13697
## What is the purpose of the change & Brief change log
- Introduce createBucketWriter to BucketsBuilder: Compaction operator depends on BucketWriter to write compacted file.
- Introduce FileLifeCycleListener to Buckets: Compaction operator depends on this to compact files in downstream operator
## Verifying this change
`BucketsTest.testFileLifeCycleListener`
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: no
- The serializers: no
- The runtime per-record code paths (performance sensitive): no
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn/Mesos, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? yes
- If yes, how is the feature documented? JavaDocs
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] gaoyunhaii commented on a change in pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
gaoyunhaii commented on a change in pull request #13697:
URL: https://github.com/apache/flink/pull/13697#discussion_r513921052
##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/filesystem/FileLifeCycleListener.java
##########
@@ -0,0 +1,41 @@
+/*
+ * 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.flink.streaming.api.functions.sink.filesystem;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.core.fs.Path;
+
+/**
+ * Listener about the status of file.
+ */
+@Internal
+public interface FileLifeCycleListener<BucketID> {
+
+ /**
+ * Notifies a new file has been opened.
+ *
+ * <p>Note that this does not mean that the file has been created in the file system. It is
+ * only created logically and the actual file will be generated after it is committed.
+ *
+ * @param bucketID The bucketID of newly opened file.
+ * @param newPath The path of newly opened file.
+ */
+ void openPartFile(BucketID bucketID, Path newPath);
Review comment:
I recommend we change the name of this method to `partFileOpened` or `onPartFileOpened`, the current name seems to open the part file actually in this method.
##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/filesystem/FileLifeCycleListener.java
##########
@@ -0,0 +1,41 @@
+/*
+ * 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.flink.streaming.api.functions.sink.filesystem;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.core.fs.Path;
+
+/**
+ * Listener about the status of file.
+ */
+@Internal
+public interface FileLifeCycleListener<BucketID> {
+
+ /**
+ * Notifies a new file has been opened.
+ *
+ * <p>Note that this does not mean that the file has been created in the file system. It is
+ * only created logically and the actual file will be generated after it is committed.
+ *
+ * @param bucketID The bucketID of newly opened file.
+ * @param newPath The path of newly opened file.
+ */
+ void openPartFile(BucketID bucketID, Path newPath);
Review comment:
I think we might change the name of this method to `partFileOpened` or `onPartFileOpened`, the current name seems to open the part file actually in this 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] JingsongLi commented on a change in pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on a change in pull request #13697:
URL: https://github.com/apache/flink/pull/13697#discussion_r513143670
##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/filesystem/FileLifeCycleListener.java
##########
@@ -0,0 +1,41 @@
+/*
+ * 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.flink.streaming.api.functions.sink.filesystem;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.core.fs.Path;
+
+/**
+ * Listener about the status of file.
+ */
+@Internal
+public interface FileLifeCycleListener<IN, BucketID> {
+
+ /**
+ * Notifies a new file has been opened.
+ *
+ * <p>Note that this does not mean that the file has been created in the file system. It is
+ * only created logically and the actual file will be generated after it is committed.
+ *
+ * @param bucket The bucket of newly opened file.
+ * @param newPath The path of newly opened file.
+ */
+ void openPartFile(Bucket<IN, BucketID> bucket, Path newPath);
Review comment:
Good point!
+1 for not exposing the part counter.
In addition, consider whether it is enough to expose only the `BucketID`? Because the new part path is already provided, I think `BucketPath` is useless in this case too.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #13697:
URL: https://github.com/apache/flink/pull/13697#issuecomment-712662619
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit b45885955bfdee4d4288f9273171e1d52379773f (Tue Oct 20 07:46:36 UTC 2020)
**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!
* **This pull request references an unassigned [Jira ticket](https://issues.apache.org/jira/browse/FLINK-19357).** According to the [code contribution guide](https://flink.apache.org/contributing/contribute-code.html), tickets need to be assigned before starting with the implementation work.
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] JingsongLi commented on pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on pull request #13697:
URL: https://github.com/apache/flink/pull/13697#issuecomment-717647349
> I had some initial comments on the PR @JingsongLi . Please let me know what you think.
Thanks a lot! I will take a look.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13697:
URL: https://github.com/apache/flink/pull/13697#issuecomment-712672697
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b45885955bfdee4d4288f9273171e1d52379773f",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7914",
"triggerID" : "b45885955bfdee4d4288f9273171e1d52379773f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* b45885955bfdee4d4288f9273171e1d52379773f Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7914)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13697:
URL: https://github.com/apache/flink/pull/13697#issuecomment-712672697
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b45885955bfdee4d4288f9273171e1d52379773f",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7914",
"triggerID" : "b45885955bfdee4d4288f9273171e1d52379773f",
"triggerType" : "PUSH"
}, {
"hash" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8490",
"triggerID" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* b45885955bfdee4d4288f9273171e1d52379773f Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7914)
* bd25177243a782fad2e7f9d9ff508e6ba3758303 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8490)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13697:
URL: https://github.com/apache/flink/pull/13697#issuecomment-712672697
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b45885955bfdee4d4288f9273171e1d52379773f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7914",
"triggerID" : "b45885955bfdee4d4288f9273171e1d52379773f",
"triggerType" : "PUSH"
}, {
"hash" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8490",
"triggerID" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"triggerType" : "PUSH"
}, {
"hash" : "48c716cf4d460cd40d7f87eed79386a09d55ed5f",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "48c716cf4d460cd40d7f87eed79386a09d55ed5f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* bd25177243a782fad2e7f9d9ff508e6ba3758303 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8490)
* 48c716cf4d460cd40d7f87eed79386a09d55ed5f UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] JingsongLi merged pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
JingsongLi merged pull request #13697:
URL: https://github.com/apache/flink/pull/13697
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13697:
URL: https://github.com/apache/flink/pull/13697#issuecomment-712672697
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b45885955bfdee4d4288f9273171e1d52379773f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7914",
"triggerID" : "b45885955bfdee4d4288f9273171e1d52379773f",
"triggerType" : "PUSH"
}, {
"hash" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8490",
"triggerID" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"triggerType" : "PUSH"
}, {
"hash" : "48c716cf4d460cd40d7f87eed79386a09d55ed5f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8563",
"triggerID" : "48c716cf4d460cd40d7f87eed79386a09d55ed5f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* bd25177243a782fad2e7f9d9ff508e6ba3758303 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8490)
* 48c716cf4d460cd40d7f87eed79386a09d55ed5f Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8563)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13697:
URL: https://github.com/apache/flink/pull/13697#issuecomment-712672697
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b45885955bfdee4d4288f9273171e1d52379773f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7914",
"triggerID" : "b45885955bfdee4d4288f9273171e1d52379773f",
"triggerType" : "PUSH"
}, {
"hash" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8490",
"triggerID" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"triggerType" : "PUSH"
}, {
"hash" : "48c716cf4d460cd40d7f87eed79386a09d55ed5f",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8563",
"triggerID" : "48c716cf4d460cd40d7f87eed79386a09d55ed5f",
"triggerType" : "PUSH"
}, {
"hash" : "5c0eba240f7daacc11e0fa08cb16cb5132873e7f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8590",
"triggerID" : "5c0eba240f7daacc11e0fa08cb16cb5132873e7f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 48c716cf4d460cd40d7f87eed79386a09d55ed5f Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8563)
* 5c0eba240f7daacc11e0fa08cb16cb5132873e7f Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8590)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13697:
URL: https://github.com/apache/flink/pull/13697#issuecomment-712672697
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b45885955bfdee4d4288f9273171e1d52379773f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7914",
"triggerID" : "b45885955bfdee4d4288f9273171e1d52379773f",
"triggerType" : "PUSH"
}, {
"hash" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8490",
"triggerID" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"triggerType" : "PUSH"
}, {
"hash" : "48c716cf4d460cd40d7f87eed79386a09d55ed5f",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8563",
"triggerID" : "48c716cf4d460cd40d7f87eed79386a09d55ed5f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 48c716cf4d460cd40d7f87eed79386a09d55ed5f Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8563)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] kl0u commented on a change in pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
kl0u commented on a change in pull request #13697:
URL: https://github.com/apache/flink/pull/13697#discussion_r512847290
##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/filesystem/FileLifeCycleListener.java
##########
@@ -0,0 +1,41 @@
+/*
+ * 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.flink.streaming.api.functions.sink.filesystem;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.core.fs.Path;
+
+/**
+ * Listener about the status of file.
+ */
+@Internal
+public interface FileLifeCycleListener<IN, BucketID> {
Review comment:
Shouldn't we also have a `closePathFile()` for symmetry?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13697:
URL: https://github.com/apache/flink/pull/13697#issuecomment-712672697
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b45885955bfdee4d4288f9273171e1d52379773f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7914",
"triggerID" : "b45885955bfdee4d4288f9273171e1d52379773f",
"triggerType" : "PUSH"
}, {
"hash" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8490",
"triggerID" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* bd25177243a782fad2e7f9d9ff508e6ba3758303 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8490)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13697:
URL: https://github.com/apache/flink/pull/13697#issuecomment-712672697
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b45885955bfdee4d4288f9273171e1d52379773f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7914",
"triggerID" : "b45885955bfdee4d4288f9273171e1d52379773f",
"triggerType" : "PUSH"
}, {
"hash" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8490",
"triggerID" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"triggerType" : "PUSH"
}, {
"hash" : "48c716cf4d460cd40d7f87eed79386a09d55ed5f",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8563",
"triggerID" : "48c716cf4d460cd40d7f87eed79386a09d55ed5f",
"triggerType" : "PUSH"
}, {
"hash" : "5c0eba240f7daacc11e0fa08cb16cb5132873e7f",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "5c0eba240f7daacc11e0fa08cb16cb5132873e7f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 48c716cf4d460cd40d7f87eed79386a09d55ed5f Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8563)
* 5c0eba240f7daacc11e0fa08cb16cb5132873e7f UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] JingsongLi commented on a change in pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on a change in pull request #13697:
URL: https://github.com/apache/flink/pull/13697#discussion_r513144820
##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/filesystem/BucketFactory.java
##########
@@ -37,6 +39,7 @@
final long initialPartCounter,
final BucketWriter<IN, BucketID> bucketWriter,
final RollingPolicy<IN, BucketID> rollingPolicy,
+ @Nullable final FileLifeCycleListener<IN, BucketID> fileListener,
Review comment:
https://flink.apache.org/contributing/code-style-and-quality-java.html#java-optional
In Flink code style, it is recommended to use the `Optional` only in method return values.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] JingsongLi commented on pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on pull request #13697:
URL: https://github.com/apache/flink/pull/13697#issuecomment-718463957
Thanks for your review, @kl0u , @gaoyunhaii !
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] kl0u commented on a change in pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
kl0u commented on a change in pull request #13697:
URL: https://github.com/apache/flink/pull/13697#discussion_r512850690
##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/filesystem/Bucket.java
##########
@@ -83,13 +86,15 @@ private Bucket(
final long initialPartCounter,
final BucketWriter<IN, BucketID> bucketWriter,
final RollingPolicy<IN, BucketID> rollingPolicy,
+ @Nullable final FileLifeCycleListener<IN, BucketID> fileListener,
Review comment:
Here also, why not `Optional`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] kl0u commented on a change in pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
kl0u commented on a change in pull request #13697:
URL: https://github.com/apache/flink/pull/13697#discussion_r512846973
##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/filesystem/FileLifeCycleListener.java
##########
@@ -0,0 +1,41 @@
+/*
+ * 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.flink.streaming.api.functions.sink.filesystem;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.core.fs.Path;
+
+/**
+ * Listener about the status of file.
+ */
+@Internal
+public interface FileLifeCycleListener<IN, BucketID> {
+
+ /**
+ * Notifies a new file has been opened.
+ *
+ * <p>Note that this does not mean that the file has been created in the file system. It is
+ * only created logically and the actual file will be generated after it is committed.
+ *
+ * @param bucket The bucket of newly opened file.
+ * @param newPath The path of newly opened file.
+ */
+ void openPartFile(Bucket<IN, BucketID> bucket, Path newPath);
Review comment:
Here we should not expose the whole `Bucket` as this is an internal class that can change anytime.
We can pass here a new interface, something like `BucketInfo`, which is going to be implemented by the `Bucket` and which only exposes the necessary information. Currently, these are the public methods in the `Bucket`, ie. the `BucketID getBucketId()`, `Path getBucketPath()`, and `long getPartCounter()`.
Actually, I would recommend to not expose the part counter because this will not be supported anymore in the new `FileSink`. If you need access to the in-progress file path, then simply pass that (the result of the `assembleNewPartPath()`) it in a field and have a method `getInProgressFilePath()` in the new `BucketInfo` interface.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13697:
URL: https://github.com/apache/flink/pull/13697#issuecomment-712672697
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b45885955bfdee4d4288f9273171e1d52379773f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7914",
"triggerID" : "b45885955bfdee4d4288f9273171e1d52379773f",
"triggerType" : "PUSH"
}, {
"hash" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8490",
"triggerID" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"triggerType" : "PUSH"
}, {
"hash" : "48c716cf4d460cd40d7f87eed79386a09d55ed5f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8563",
"triggerID" : "48c716cf4d460cd40d7f87eed79386a09d55ed5f",
"triggerType" : "PUSH"
}, {
"hash" : "5c0eba240f7daacc11e0fa08cb16cb5132873e7f",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8590",
"triggerID" : "5c0eba240f7daacc11e0fa08cb16cb5132873e7f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5c0eba240f7daacc11e0fa08cb16cb5132873e7f Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8590)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] JingsongLi commented on a change in pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on a change in pull request #13697:
URL: https://github.com/apache/flink/pull/13697#discussion_r513140384
##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/filesystem/FileLifeCycleListener.java
##########
@@ -0,0 +1,41 @@
+/*
+ * 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.flink.streaming.api.functions.sink.filesystem;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.core.fs.Path;
+
+/**
+ * Listener about the status of file.
+ */
+@Internal
+public interface FileLifeCycleListener<IN, BucketID> {
Review comment:
Yes, it's better to have `closePathFile()`.
But at present, because `InProgressFileWriter`s hide the path, we can't get the path when close file.
What's more, the `RecoverableFsDataOutputStream` recovered from the state does not save path information, such as`S3RecoverableFsDataOutputStream`, which may require us to save additional path information in state or have the ability to recover path information from the internal structure of `RecoverableFsDataOutputStream`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] JingsongLi commented on a change in pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on a change in pull request #13697:
URL: https://github.com/apache/flink/pull/13697#discussion_r513140384
##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/filesystem/FileLifeCycleListener.java
##########
@@ -0,0 +1,41 @@
+/*
+ * 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.flink.streaming.api.functions.sink.filesystem;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.core.fs.Path;
+
+/**
+ * Listener about the status of file.
+ */
+@Internal
+public interface FileLifeCycleListener<IN, BucketID> {
Review comment:
Yes, it's better to have `closePathFile()`.
But at present, because `InProgressFileWriter`s hide the path, we can't get the path when close file.
What's more, the `RecoverableFsDataOutputStream` recovered from the state does not save path information, such as`S3RecoverableFsDataOutputStream`, which may require us to save additional path information or have the ability to recover path information from the internal structure of `RecoverableFsDataOutputStream`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13697:
URL: https://github.com/apache/flink/pull/13697#issuecomment-712672697
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b45885955bfdee4d4288f9273171e1d52379773f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7914",
"triggerID" : "b45885955bfdee4d4288f9273171e1d52379773f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* b45885955bfdee4d4288f9273171e1d52379773f Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7914)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #13697:
URL: https://github.com/apache/flink/pull/13697#issuecomment-712672697
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b45885955bfdee4d4288f9273171e1d52379773f",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "b45885955bfdee4d4288f9273171e1d52379773f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* b45885955bfdee4d4288f9273171e1d52379773f UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13697:
URL: https://github.com/apache/flink/pull/13697#issuecomment-712672697
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b45885955bfdee4d4288f9273171e1d52379773f",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7914",
"triggerID" : "b45885955bfdee4d4288f9273171e1d52379773f",
"triggerType" : "PUSH"
}, {
"hash" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "bd25177243a782fad2e7f9d9ff508e6ba3758303",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* b45885955bfdee4d4288f9273171e1d52379773f Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7914)
* bd25177243a782fad2e7f9d9ff508e6ba3758303 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] JingsongLi commented on a change in pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on a change in pull request #13697:
URL: https://github.com/apache/flink/pull/13697#discussion_r513147361
##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/filesystem/StreamingFileSink.java
##########
@@ -251,14 +254,20 @@ T withBucketFactory(final BucketFactory<IN, BucketID> factory) {
return self();
}
+ @Internal
+ @Override
Review comment:
Sorry, let me explain the context:
In the file compaction, we need to write a file with the specified name. We hope to use `BucketWriter` to complete atomic file level writing.
The `BucketWriter` can complete atomic writing, but also shield the implementation details of BulkWriter, RowWiseWriter, OutputStreamBasedWriting and PathBasedWriting. That's great.
It's like the `createBuckets` method, We can not let it be package-private, because it is used by another module (flink-table).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] kl0u commented on a change in pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
kl0u commented on a change in pull request #13697:
URL: https://github.com/apache/flink/pull/13697#discussion_r512850046
##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/filesystem/BucketFactory.java
##########
@@ -37,6 +39,7 @@
final long initialPartCounter,
final BucketWriter<IN, BucketID> bucketWriter,
final RollingPolicy<IN, BucketID> rollingPolicy,
+ @Nullable final FileLifeCycleListener<IN, BucketID> fileListener,
Review comment:
Why not having an `Optional` instead of a `@Nullable`? I think `null` leads to more "fragile" code and more difficult to maintain.
##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/filesystem/BucketFactory.java
##########
@@ -45,5 +48,6 @@
final BucketWriter<IN, BucketID> bucketWriter,
final RollingPolicy<IN, BucketID> rollingPolicy,
final BucketState<BucketID> bucketState,
+ @Nullable final FileLifeCycleListener<IN, BucketID> fileListener,
Review comment:
Same as above.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] kl0u commented on a change in pull request #13697: [FLINK-19357][FLINK-19357][fs-connector] Introduce createBucketWriter to BucketsBuilder & Introduce FileLifeCycleListener to Buckets
Posted by GitBox <gi...@apache.org>.
kl0u commented on a change in pull request #13697:
URL: https://github.com/apache/flink/pull/13697#discussion_r512853491
##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/filesystem/StreamingFileSink.java
##########
@@ -251,14 +254,20 @@ T withBucketFactory(final BucketFactory<IN, BucketID> factory) {
return self();
}
+ @Internal
+ @Override
Review comment:
@JingsongLi could you describe how this method is going to be used? Because in this commit, it seems that it is a private method of the builder :) Could it at least be package-private so that it is not exposed to the user?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org