You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "showuon (via GitHub)" <gi...@apache.org> on 2023/09/12 09:06:32 UTC
[GitHub] [kafka] showuon opened a new pull request, #14373: KAFKA-15453: enable testFencingOnTransactionExpiration
showuon opened a new pull request, #14373:
URL: https://github.com/apache/kafka/pull/14373
waiting for https://github.com/apache/kafka/pull/14347 .
==
This PR did:
1. enable `testFencingOnTransactionExpiration`
2. Verify the log start offset and local log start offset before and after committing/aborting txn, to make sure we will move the segment to remote storage after last stable offset advanced.
For (1), the reason the test failed before because:
1. RLMM is not ready since `remote.log.metadata.initialization.retry.interval.ms` is too long to wait for retrying initialization. Reducing it to 200 ms to fix the issue.
2. Even though we extends the `TransactionsTest`, we didn't inherit some changes in the class initialization. And that causes the test doesn't run as we expected, ex: `TransactionsTopicPartitionsProp` is using 50 in tiered storage case, but it should be 3... etc. Fixed it by inherit all configs in parent class now.
### 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] satishd commented on pull request #14373: KAFKA-15453: enable testFencingOnTransactionExpiration
Posted by "satishd (via GitHub)" <gi...@apache.org>.
satishd commented on PR #14373:
URL: https://github.com/apache/kafka/pull/14373#issuecomment-1728982915
@showuon I think you have already pushed the changes to another PR that got merged. Are there any more changes planned as part of 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] satishd commented on pull request #14373: KAFKA-15453: enable testFencingOnTransactionExpiration
Posted by "satishd (via GitHub)" <gi...@apache.org>.
satishd commented on PR #14373:
URL: https://github.com/apache/kafka/pull/14373#issuecomment-1715566686
Nice catch @showuon !!
--
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] kamalcph commented on pull request #14373: KAFKA-15453: enable testFencingOnTransactionExpiration
Posted by "kamalcph (via GitHub)" <gi...@apache.org>.
kamalcph commented on PR #14373:
URL: https://github.com/apache/kafka/pull/14373#issuecomment-1715462178
@showuon
> Even though we extends the TransactionsTest, we didn't inherit some changes in the class initialization. And that causes the test doesn't run as we expected, ex: TransactionsTopicPartitionsProp is using 50 in tiered storage case, but it should be 3... etc. Fixed it by inherit all configs in parent class now.
The behaviour is same in our internal branch. While moving the code, I missed it. Thanks for fixing it! I've invited you as collaborator to my branch. You can overlay this change on top of #14347, so that all the tests can be verified in one 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] showuon commented on pull request #14373: KAFKA-15453: enable testFencingOnTransactionExpiration
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon commented on PR #14373:
URL: https://github.com/apache/kafka/pull/14373#issuecomment-1729025051
Close it since it's been addressed in another 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] showuon closed pull request #14373: KAFKA-15453: enable testFencingOnTransactionExpiration
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon closed pull request #14373: KAFKA-15453: enable testFencingOnTransactionExpiration
URL: https://github.com/apache/kafka/pull/14373
--
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] showuon commented on pull request #14373: KAFKA-15453: enable testFencingOnTransactionExpiration
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon commented on PR #14373:
URL: https://github.com/apache/kafka/pull/14373#issuecomment-1715543328
> The behaviour is same in our internal branch. While moving the code, I missed it. Thanks for fixing it! I've invited you as collaborator to my branch. You can overlay this change on top of https://github.com/apache/kafka/pull/14347, so that all the tests can be verified in one PR. Added you as the co-author of https://github.com/apache/kafka/pull/14347.
That's great! But I'd prefer separate these changes as a separate PR since https://github.com/apache/kafka/pull/14347 had completed reviewed and got 3 approvals. We should keep it as is and merge it first, then have a new review in 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] showuon commented on pull request #14373: KAFKA-15453: enable testFencingOnTransactionExpiration
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon commented on PR #14373:
URL: https://github.com/apache/kafka/pull/14373#issuecomment-1715545842
Oh wait, I saw there are many failed transaction tests and need my change here. OK, I'll cherry-pick into your branch later (after dinner, sorry).
--
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