You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "ljz2051 (via GitHub)" <gi...@apache.org> on 2024/01/10 02:25:26 UTC
[PR] [FLINK-33881]Avoid copy and update value in TtlListState#getUnexpiredOrNull [flink]
ljz2051 opened a new pull request, #24057:
URL: https://github.com/apache/flink/pull/24057
## What is the purpose of the change
This pull request optimize TtlListState clean up process by avoiding copy and update value in TtlListState#getUnexpiredOrNull
## Brief change log
- refactor TtlListState#getUnexpiredOrNull
## Verifying this change
This change is already covered by existing tests, such as *(please describe tests)*.
org.apache.flink.runtime.state.ttl.TtlStateTestBase + TtlFixedLenElemListStateTestContext/TtlNonFixedLenElemListStateTestContext
## 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, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? no
--
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
Re: [PR] [FLINK-33881]Avoid copy and update value in TtlListState#getUnexpiredOrNull [flink]
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
masteryhx commented on PR #24057:
URL: https://github.com/apache/flink/pull/24057#issuecomment-1886347612
merged.
Just a reminder, remeber to write commit message with component tag next time.
--
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
Re: [PR] [FLINK-33881]Avoid copy and update value in TtlListState#getUnexpiredOrNull [flink]
Posted by "ljz2051 (via GitHub)" <gi...@apache.org>.
ljz2051 commented on code in PR #24057:
URL: https://github.com/apache/flink/pull/24057#discussion_r1447307735
##########
flink-runtime/src/main/java/org/apache/flink/runtime/state/ttl/TtlListState.java:
##########
@@ -109,14 +109,32 @@ public List<TtlValue<T>> getUnexpiredOrNull(@Nonnull List<TtlValue<T>> ttlValues
}
long currentTimestamp = timeProvider.currentTimestamp();
- List<TtlValue<T>> unexpired = new ArrayList<>(ttlValues.size());
TypeSerializer<TtlValue<T>> elementSerializer =
((ListSerializer<TtlValue<T>>) original.getValueSerializer())
.getElementSerializer();
- for (TtlValue<T> ttlValue : ttlValues) {
- if (!TtlUtils.expired(ttlValue, ttl, currentTimestamp)) {
+ int firstExpireElementIndex = -1;
+ for (int i = 0; i < ttlValues.size(); i++) {
+ TtlValue<T> ttlValue = ttlValues.get(i);
+ if (TtlUtils.expired(ttlValue, ttl, currentTimestamp)) {
+ firstExpireElementIndex = i;
+ break;
+ }
+ }
+ if (firstExpireElementIndex == -1) {
+ return ttlValues;
+ }
+
+ List<TtlValue<T>> unexpired = new ArrayList<>(ttlValues.size());
+ for (int i = 0; i < ttlValues.size(); i++) {
+ TtlValue<T> ttlValue = ttlValues.get(i);
+ if (i < firstExpireElementIndex) {
// we have to do the defensive copy to update the value
unexpired.add(elementSerializer.copy(ttlValue));
+ } else if (i > firstExpireElementIndex) {
+ if (!TtlUtils.expired(ttlValue, ttl, currentTimestamp)) {
Review Comment:
Good idea. I've merged it.
--
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
Re: [PR] [FLINK-33881]Avoid copy and update value in TtlListState#getUnexpiredOrNull [flink]
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
masteryhx closed pull request #24057: [FLINK-33881]Avoid copy and update value in TtlListState#getUnexpiredOrNull
URL: https://github.com/apache/flink/pull/24057
--
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
Re: [PR] [FLINK-33881]Avoid copy and update value in TtlListState#getUnexpiredOrNull [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #24057:
URL: https://github.com/apache/flink/pull/24057#issuecomment-1884093107
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b1eef2343133eedbae3219ff1ecfe9984e846769",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "b1eef2343133eedbae3219ff1ecfe9984e846769",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* b1eef2343133eedbae3219ff1ecfe9984e846769 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-33881]Avoid copy and update value in TtlListState#getUnexpiredOrNull [flink]
Posted by "Zakelly (via GitHub)" <gi...@apache.org>.
Zakelly commented on code in PR #24057:
URL: https://github.com/apache/flink/pull/24057#discussion_r1447239317
##########
flink-runtime/src/main/java/org/apache/flink/runtime/state/ttl/TtlListState.java:
##########
@@ -109,14 +109,32 @@ public List<TtlValue<T>> getUnexpiredOrNull(@Nonnull List<TtlValue<T>> ttlValues
}
long currentTimestamp = timeProvider.currentTimestamp();
- List<TtlValue<T>> unexpired = new ArrayList<>(ttlValues.size());
TypeSerializer<TtlValue<T>> elementSerializer =
((ListSerializer<TtlValue<T>>) original.getValueSerializer())
.getElementSerializer();
- for (TtlValue<T> ttlValue : ttlValues) {
- if (!TtlUtils.expired(ttlValue, ttl, currentTimestamp)) {
+ int firstExpireElementIndex = -1;
+ for (int i = 0; i < ttlValues.size(); i++) {
+ TtlValue<T> ttlValue = ttlValues.get(i);
+ if (TtlUtils.expired(ttlValue, ttl, currentTimestamp)) {
+ firstExpireElementIndex = i;
+ break;
+ }
+ }
+ if (firstExpireElementIndex == -1) {
+ return ttlValues;
+ }
+
+ List<TtlValue<T>> unexpired = new ArrayList<>(ttlValues.size());
+ for (int i = 0; i < ttlValues.size(); i++) {
+ TtlValue<T> ttlValue = ttlValues.get(i);
+ if (i < firstExpireElementIndex) {
// we have to do the defensive copy to update the value
unexpired.add(elementSerializer.copy(ttlValue));
+ } else if (i > firstExpireElementIndex) {
+ if (!TtlUtils.expired(ttlValue, ttl, currentTimestamp)) {
Review Comment:
How about merge these branches into one?
```
if (i < firstExpireElementIndex || (i > firstExpireElementIndex && !TtlUtils.expired(ttlValue, ttl, currentTimestamp))) {
//...
}
```
--
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