You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/07/15 18:26:54 UTC
[GitHub] [kafka] tang7526 opened a new pull request #11062: KAFKA-13094: Session windows do not consider user-specified grace when computing retention time for changelog
tang7526 opened a new pull request #11062:
URL: https://github.com/apache/kafka/pull/11062
https://issues.apache.org/jira/browse/KAFKA-13094
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] guozhangwang commented on a change in pull request #11062: KAFKA-13094: Session windows do not consider user-specified grace when computing retention time for changelog
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on a change in pull request #11062:
URL: https://github.com/apache/kafka/pull/11062#discussion_r671425031
##########
File path: streams/src/main/java/org/apache/kafka/streams/kstream/SessionWindows.java
##########
@@ -187,7 +187,8 @@ public long inactivityGap() {
*/
@Deprecated
public long maintainMs() {
- return Math.max(maintainDurationMs, gapMs);
+ final long gracePeriodMs = graceMs != -1 ? graceMs : Math.max(maintainDurationMs, gapMs) - inactivityGap();
Review comment:
This is not fixing the issue.. we should use the `graceMs` here, which may be specified by 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tang7526 commented on pull request #11062: KAFKA-13094: Session windows do not consider user-specified grace when computing retention time for changelog
Posted by GitBox <gi...@apache.org>.
tang7526 commented on pull request #11062:
URL: https://github.com/apache/kafka/pull/11062#issuecomment-880920746
@cadonna Could you help to review this PR? thanks.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cadonna commented on pull request #11062: KAFKA-13094: Session windows do not consider user-specified grace when computing retention time for changelog
Posted by GitBox <gi...@apache.org>.
cadonna commented on pull request #11062:
URL: https://github.com/apache/kafka/pull/11062#issuecomment-883565851
@tang7526 Could you wait until #11061 is merged and rebase this PR on 2.8 afterwards to get all unit tests that are important for this PR?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tang7526 commented on pull request #11062: KAFKA-13094: Session windows do not consider user-specified grace when computing retention time for changelog
Posted by GitBox <gi...@apache.org>.
tang7526 commented on pull request #11062:
URL: https://github.com/apache/kafka/pull/11062#issuecomment-883578011
> @tang7526 Could you wait until #11061 is merged and rebase this PR on 2.8 afterwards to get all unit tests that are important for this PR?
@cadonna Copy that.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] guozhangwang commented on a change in pull request #11062: KAFKA-13094: Session windows do not consider user-specified grace when computing retention time for changelog
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on a change in pull request #11062:
URL: https://github.com/apache/kafka/pull/11062#discussion_r673324333
##########
File path: streams/src/main/java/org/apache/kafka/streams/kstream/SessionWindows.java
##########
@@ -187,7 +187,8 @@ public long inactivityGap() {
*/
@Deprecated
public long maintainMs() {
- return Math.max(maintainDurationMs, gapMs);
+ final long gracePeriodMs = graceMs != -1 ? graceMs : Math.max(maintainDurationMs, gapMs) - inactivityGap();
Review comment:
Yes, that's right.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tang7526 commented on pull request #11062: KAFKA-13094: Session windows do not consider user-specified grace when computing retention time for changelog
Posted by GitBox <gi...@apache.org>.
tang7526 commented on pull request #11062:
URL: https://github.com/apache/kafka/pull/11062#issuecomment-885088407
> @tang7526 Could you rebase this PR on the latest 2.8 branch?
@cadonna Done. I've already rebased this PR on the latest 2.8 branch.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tang7526 commented on a change in pull request #11062: KAFKA-13094: Session windows do not consider user-specified grace when computing retention time for changelog
Posted by GitBox <gi...@apache.org>.
tang7526 commented on a change in pull request #11062:
URL: https://github.com/apache/kafka/pull/11062#discussion_r671589951
##########
File path: streams/src/main/java/org/apache/kafka/streams/kstream/SessionWindows.java
##########
@@ -187,7 +187,8 @@ public long inactivityGap() {
*/
@Deprecated
public long maintainMs() {
- return Math.max(maintainDurationMs, gapMs);
+ final long gracePeriodMs = graceMs != -1 ? graceMs : Math.max(maintainDurationMs, gapMs) - inactivityGap();
Review comment:
@guozhangwang Thank you for your guidance.
So I should change it to the following code. Is this correct?
```
public long maintainMs() {
return Math.max(maintainDurationMs, gapMs + graceMs);
}
```
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cadonna edited a comment on pull request #11062: KAFKA-13094: Session windows do not consider user-specified grace when computing retention time for changelog
Posted by GitBox <gi...@apache.org>.
cadonna edited a comment on pull request #11062:
URL: https://github.com/apache/kafka/pull/11062#issuecomment-885080580
@tang7526 Could you rebase this PR on the latest 2.8 branch?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] guozhangwang commented on pull request #11062: KAFKA-13094: Session windows do not consider user-specified grace when computing retention time for changelog
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on pull request #11062:
URL: https://github.com/apache/kafka/pull/11062#issuecomment-883560474
Also cc @cadonna for a final review and merge.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cadonna commented on pull request #11062: KAFKA-13094: Session windows do not consider user-specified grace when computing retention time for changelog
Posted by GitBox <gi...@apache.org>.
cadonna commented on pull request #11062:
URL: https://github.com/apache/kafka/pull/11062#issuecomment-885080580
@tang7526 Could you rebase this PR on the latest 2.8 branch.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org