[fix][broker] change name limitTime to limitTimeInSec

[StevenLuMT]

Motivation

limitTime It's a confusing name, without the time unit, it is easy to cause bugs,
so we should deprecate the old name limitTime and introduce a new one limitTimeInSec

Modifications

change name limitTime to limitTimeInSec

Verifying this change

• Make sure that the change passes the CI checks.
  This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

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

Check the box below or label this PR directly.

Need to update docs?

• doc
• doc-required
• doc-not-needed
• doc-complete

Matching PR in forked repository

PR in forked repository: StevenLuMT#1

[github-actions]

@StevenLuMT 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 -->

[HQebupt]

Good catch. LGTM

[codecov-commenter]

Codecov Report

Merging #19053 (3d6f001) into master (d8569cd) will decrease coverage by 2.14%.
The diff coverage is 33.33%.

@@             Coverage Diff              @@
##             master   #19053      +/-   ##
============================================
- Coverage     47.19%   45.05%   -2.15%     
- Complexity    10643    10826     +183     
============================================
  Files           709      769      +60     
  Lines         69421    74178    +4757     
  Branches       7449     7982     +533     
============================================
+ Hits          32766    33421     +655     
- Misses        32986    36986    +4000     
- Partials       3669     3771     +102     

Flag	Coverage Δ
unittests	45.05% <33.33%> (-2.15%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...che/pulsar/broker/service/BacklogQuotaManager.java	12.39% <0.00%> (ø)
...ker/stats/prometheus/NamespaceStatsAggregator.java	0.00% <0.00%> (ø)
...sar/broker/service/persistent/PersistentTopic.java	60.70% <50.00%> (-0.85%)	⬇️
.../org/apache/pulsar/broker/admin/AdminResource.java	66.58% <100.00%> (ø)
.../pulsar/broker/service/SharedConsumerAssignor.java	3.70% <0.00%> (-64.82%)	⬇️
...java/org/apache/pulsar/proxy/stats/TopicStats.java	58.82% <0.00%> (-41.18%)	⬇️
...apache/pulsar/broker/service/EntryAndMetadata.java	0.00% <0.00%> (-40.75%)	⬇️
...rg/apache/pulsar/broker/lookup/v1/TopicLookup.java	60.00% <0.00%> (-13.34%)	⬇️
...e/pulsar/broker/service/EntryBatchIndexesAcks.java	82.14% <0.00%> (-10.72%)	⬇️
...roker/service/persistent/MessageDeduplication.java	43.23% <0.00%> (-7.43%)	⬇️
... and 90 more

[michaeljmarshall]

@yuruguo @StevenLuMT - this is a breaking change without accounting for backwards compatibility. We either need to revert this or do something like #13291 (I haven't looked closely at that PR, but I think it is the right reference).

Note that this type of API change is technically supposed to go through the PIP process.

This type of change has far reaching implications for clients, too, so there must be new documentation for the change.

Let me know how you'd like to proceed.

[michaeljmarshall]

This is a breaking change without any backwards compatibility.

[lhotari]

I think it's better to revert this change and instead focus on improving documentation.

[michaeljmarshall]

I agree with that approach. There are many names in Pulsar that are slightly confusing. It'd make more sense to define a paradigm for naming before making many one-off changes.

[michaeljmarshall]

Reverting with #19152. I support working to improve the clarify of our names, but we need to be careful not to break any deployments.

[Jason918]

@StevenLuMT The right way is to keep compatibility, you can take the old getLimit for reference.

1. add getLimitTimeInSec in BacklogQuota
2. Deprecate getLimitTime in BacklogQuota
3. keep getLimitTime and setLimitTime in BacklogQuotaImpl.