Skip to content

Fix incompatibility of BacklogQuota - #13291

Merged
eolivelli merged 2 commits into
apache:masterfrom
hezhangjian:fix-incompatibility-of-backlogquota
Dec 15, 2021
Merged

Fix incompatibility of BacklogQuota#13291
eolivelli merged 2 commits into
apache:masterfrom
hezhangjian:fix-incompatibility-of-backlogquota

Conversation

@hezhangjian

Copy link
Copy Markdown
Member

Motivation

there is a change between pulsar2.7 and master
command

bin/pulsar admin namespaces get-backlog-quotas $tenant/$namespace

pulsar2.7 returns

destination_storage    BacklogQuotaImpl(limit=102400, limitTime=-1, policy=consumer_backlog_eviction)

but pulsar 2.9 returns

"destination_storage    BacklogQuotaImpl(limitSize=102400, limitTime=-1, policy=consumer_backlog_eviction)"

So we need a compat, let old REST/sdk user, can set and get limit

Verifying this change

  • add org.apache.pulsar.broker.resources.NamespaceBacklogQuotaCompatibleTest for test

Documentation

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

  • no-need-doc

It's a compatibility bug fix.

@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Dec 14, 2021
@merlimat merlimat added this to the 2.10.0 milestone Dec 14, 2021
@hezhangjian
hezhangjian force-pushed the fix-incompatibility-of-backlogquota branch from a862901 to 4723794 Compare December 14, 2021 15:13
@hezhangjian hezhangjian changed the title Fix incompatibility of BacklogQuota Fix incompatibility of GetBacklogQuota Dec 14, 2021
@codelipenghui

Copy link
Copy Markdown
Contributor

@shoothzj I noticed the fix only fixes the compatibility issue when getting backlog quota, I think another compatibility issue is when using a new version client >= 2.8 but the broker which < 2.8, since the broker-side don't have limitSize and the client side will not pass the limit to the broker, so that the broker will use 0 as the value of value, it also should be a compatibility issue.

It's better to add a compatibility test to make sure we will not introduce breaking change for this part.

Thanks for the great work.

@hezhangjian hezhangjian changed the title Fix incompatibility of GetBacklogQuota Fix incompatibility of BacklogQuota Dec 15, 2021
@hezhangjian
hezhangjian force-pushed the fix-incompatibility-of-backlogquota branch from 4723794 to c81537a Compare December 15, 2021 00:41
@hezhangjian

Copy link
Copy Markdown
Member Author

@codelipenghui PTAL again, thanks

@hezhangjian

Copy link
Copy Markdown
Member Author

@eolivelli @linlinnn PTAL, and should this cherry pick to 2.8 and 2.9 ? thanks

@eolivelli

Copy link
Copy Markdown
Contributor

Are we breaking compatibility with 2.8 ?

2.7 is far away, and the current release is 2.8...
I wonder if it is better to restore compatibility with 2.7 and break against 2.8 or is it better to keep compatibility with 2.8 and add another point to the "breaking changes" ?

because IIUC we broke compatibility in 2.8 and now (we are at 2.9) it is too late

@eolivelli

Copy link
Copy Markdown
Contributor

I have now understood better the final outcome of this patch.
I will cherry pick to branch-2.9 and prepare (again) 2.9.1

@eolivelli
eolivelli merged commit 4e89c2b into apache:master Dec 15, 2021
@hezhangjian
hezhangjian deleted the fix-incompatibility-of-backlogquota branch December 15, 2021 11:16
eolivelli pushed a commit that referenced this pull request Dec 15, 2021
@eolivelli eolivelli added cherry-picked/branch-2.9 Archived: 2.9 is end of life release/2.9.1 labels Dec 15, 2021
315157973 pushed a commit to 315157973/pulsar that referenced this pull request Dec 16, 2021
# Conflicts:
#	pulsar-common/src/test/java/org/apache/pulsar/common/util/ObjectMapperFactoryTest.java
#	pulsar-metadata/src/test/java/org/apache/pulsar/metadata/BacklogQuotaCompatibilityTest.java
315157973 added a commit that referenced this pull request Dec 16, 2021
* Fix incompatibility of BacklogQuota (#13291)

# Conflicts:
#	pulsar-common/src/test/java/org/apache/pulsar/common/util/ObjectMapperFactoryTest.java
#	pulsar-metadata/src/test/java/org/apache/pulsar/metadata/BacklogQuotaCompatibilityTest.java

* Fix jar mentioned in lib/presto/LICENSE, but not bundled

* Fix jar mentioned in lib/presto/LICENSE, but not bundled

Co-authored-by: ZhangJian He <shoothzj@gmail.com>
@315157973 315157973 added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Dec 16, 2021
fxbing pushed a commit to fxbing/pulsar that referenced this pull request Dec 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.2 release/2.9.1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants