You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "thetumbled (via GitHub)" <gi...@apache.org> on 2024/02/27 09:07:36 UTC
[PR] [fix] [branch-3.0] Fix exception in TopicTransactionBufferTest [pulsar]
thetumbled opened a new pull request, #22130:
URL: https://github.com/apache/pulsar/pull/22130
### Motivation
Exception is found in branch 3.0:
https://github.com/apache/pulsar/pull/22024#issuecomment-1965921600
### Modifications
The root is that we do not cherry pick https://github.com/apache/pulsar/pull/21466 into branch 3.0, and the test code rely on this pr.
Moreover, the test code in https://github.com/apache/pulsar/pull/21466 is merged into branch 3.0 along with my pr https://github.com/apache/pulsar/pull/22024.
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
*(Please pick either of the following options)*
This change is already covered by existing tests, such as *(please describe tests)*.
### Does this pull request potentially affect one of the following parts:
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
*If the box was checked, please highlight the changes*
- [ ] Dependencies (add or upgrade a dependency)
- [ ] The public API
- [ ] The schema
- [ ] The default values of configurations
- [ ] The threading model
- [ ] The binary protocol
- [ ] The REST endpoints
- [ ] The admin CLI options
- [ ] The metrics
- [ ] Anything that affects deployment
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
### Matching PR in forked repository
PR in forked repository: <!-- ENTER URL 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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix] [branch-3.0] Fix exception in TopicTransactionBufferTest [pulsar]
Posted by "thetumbled (via GitHub)" <gi...@apache.org>.
thetumbled commented on PR #22130:
URL: https://github.com/apache/pulsar/pull/22130#issuecomment-1966104155
PTAL, thanks.
@lhotari @Technoboy-
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [cherry-pick] [branch-3.0] Fix getting last message ID when there are ongoing transactions [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari closed pull request #22130: [cherry-pick] [branch-3.0] Fix getting last message ID when there are ongoing transactions
URL: https://github.com/apache/pulsar/pull/22130
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [cherry-pick] [branch-3.0] Fix getting last message ID when there are ongoing transactions [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari closed pull request #22130: [cherry-pick] [branch-3.0] Fix getting last message ID when there are ongoing transactions
URL: https://github.com/apache/pulsar/pull/22130
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [cherry-pick] [branch-3.0] Fix getting last message ID when there are ongoing transactions [pulsar]
Posted by "thetumbled (via GitHub)" <gi...@apache.org>.
thetumbled commented on PR #22130:
URL: https://github.com/apache/pulsar/pull/22130#issuecomment-1966223205
> let's first cherry-pick #21466
may be change the title is enough?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [cherry-pick] [branch-3.0] Fix getting last message ID when there are ongoing transactions [pulsar]
Posted by "thetumbled (via GitHub)" <gi...@apache.org>.
thetumbled commented on PR #22130:
URL: https://github.com/apache/pulsar/pull/22130#issuecomment-1968319314
Some test do not pass:
`mvn -B -T 1C -ntp -Pcore-modules,-main clean install -DskipTests -Dlicense.skip=true -Drat.skip=true -Dcheckstyle.skip=true`
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdNamespaces.java
<img width="759" alt="image" src="https://github.com/apache/pulsar/assets/52550727/c7f9416e-343f-4da2-8f69-1412d09a8f3e">
TimeUnitToSecondsConverter.class is introduced by https://github.com/apache/pulsar/pull/20782, but we do not cherry pick it into branch 3.0.
However, https://github.com/apache/pulsar/pull/22101 is cherry picked into branch-3.0, which rely on TimeUnitToSecondsConverter.class.
So the solution is: first, cherry pick https://github.com/apache/pulsar/pull/20782 into branch-3.0, then we use this pr https://github.com/apache/pulsar/pull/22143 to fix the problem.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [cherry-pick] [branch-3.0] Fix getting last message ID when there are ongoing transactions [pulsar]
Posted by "heesung-sn (via GitHub)" <gi...@apache.org>.
heesung-sn commented on PR #22130:
URL: https://github.com/apache/pulsar/pull/22130#issuecomment-1973844882
I don't see this change in this cherry-pick PR.
https://github.com/apache/pulsar/pull/21466/files#diff-ecd728301a585f256e8a649b5e65b28c166194477355b3a1eefc198d014c25d3R450
@thetumbled can you check again?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [cherry-pick] [branch-3.0] Fix getting last message ID when there are ongoing transactions [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #22130:
URL: https://github.com/apache/pulsar/pull/22130#issuecomment-1968228363
Closing and reopening as an attempt to trigger the CI builds to run. For some reason, the build didn't get triggered.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [cherry-pick] [branch-3.0] Fix getting last message ID when there are ongoing transactions [pulsar]
Posted by "thetumbled (via GitHub)" <gi...@apache.org>.
thetumbled commented on PR #22130:
URL: https://github.com/apache/pulsar/pull/22130#issuecomment-1974245344
> I don't see this change in this cherry-pick PR.
>
> https://github.com/apache/pulsar/pull/21466/files#diff-ecd728301a585f256e8a649b5e65b28c166194477355b3a1eefc198d014c25d3R450
>
> @thetumbled can you check again?
We have cherry pick pr
> I don't see this change in this cherry-pick PR.
>
> https://github.com/apache/pulsar/pull/21466/files#diff-ecd728301a585f256e8a649b5e65b28c166194477355b3a1eefc198d014c25d3R450
>
> @thetumbled can you check again?
Sorry for missing it. PTAL, 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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [cherry-pick] [branch-3.0] Fix getting last message ID when there are ongoing transactions [pulsar]
Posted by "Technoboy- (via GitHub)" <gi...@apache.org>.
Technoboy- commented on PR #22130:
URL: https://github.com/apache/pulsar/pull/22130#issuecomment-1966247328
> @Technoboy- I think we should cherry-pick #21466 first , that will ease tracking in the future
Can't cherry-pick directly. Using this and comment on the #21466 is enough
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix] [branch-3.0] Fix exception in TopicTransactionBufferTest [pulsar]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #22130:
URL: https://github.com/apache/pulsar/pull/22130#issuecomment-1966088807
@thetumbled Please add the following content to your PR description and select a checkbox:
```
- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
```
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix] [branch-3.0] Fix exception in TopicTransactionBufferTest [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #22130:
URL: https://github.com/apache/pulsar/pull/22130#issuecomment-1966193745
@Technoboy- I think we should cherry-pick #21466 first , that will ease tracking in the future
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [cherry-pick] [branch-3.0] Fix getting last message ID when there are ongoing transactions [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #22130:
URL: https://github.com/apache/pulsar/pull/22130#issuecomment-1977569961
Thank you @thetumbled for this PR. I ended up using your merge conflict resolution into this commit: 4489d03f .
I hope we find a way to fix our process that we don't end up in this problems that often in the future. I have started a discussion thread on the dev mailing list: https://lists.apache.org/thread/j6qvt45rndnvjypcmqxsfmddqt41bxjv
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [cherry-pick] [branch-3.0] Fix getting last message ID when there are ongoing transactions [pulsar]
Posted by "thetumbled (via GitHub)" <gi...@apache.org>.
thetumbled commented on PR #22130:
URL: https://github.com/apache/pulsar/pull/22130#issuecomment-1972347397
> Some test do not pass: `mvn -B -T 1C -ntp -Pcore-modules,-main clean install -DskipTests -Dlicense.skip=true -Drat.skip=true -Dcheckstyle.skip=true`
>
> ```
> cannot find symbol
> Error: symbol: class TimeUnitToSecondsConverter
> Error: location: class org.apache.pulsar.admin.cli.CmdNamespaces.SetOffloadThreshold
> ```
>
> <img alt="image" width="759" src="https://private-user-images.githubusercontent.com/52550727/308443300-c7f9416e-343f-4da2-8f69-1412d09a8f3e.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MDkyNjA0ODgsIm5iZiI6MTcwOTI2MDE4OCwicGF0aCI6Ii81MjU1MDcyNy8zMDg0NDMzMDAtYzdmOTQxNmUtMzQzZi00ZGEyLThmNjktMTQxMmQwOWE4ZjNlLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDAzMDElMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwMzAxVDAyMjk0OFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTY4M2Q2YjI1MWFmZGIwNGQ1Mjc4ZDkyMDAyMjEzZmRkMzNlOTFlYWNmYzRlNmFkMmI4ZTUwZDQxM2FhZTMyMzAmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.7AJFmFK2emFb1Sewjqw_tMW98AgNs_KdEY3YbUm1eyM">
> TimeUnitToSecondsConverter.class is introduced by #20782 , but we do not cherry pick it into branch 3.0. However, #22101 is cherry picked into branch-3.0, which rely on TimeUnitToSecondsConverter.class.
>
> So the solution is: first, cherry pick #20782 into branch-3.0, then we use this pr #22143 to fix the problem. PTAL, thanks. @lhotari @Technoboy-
The problem has been fixed by https://github.com/apache/pulsar/pull/22169, we can proceed with reviewing this pr. @lhotari @heesung-sn
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org