Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't create topics after #12818 using Pulsar master client and 2.9.x (or prior) broker (v3/persistent NOT FOUND) #14104

Closed
nicoloboschi opened this issue Feb 3, 2022 · 8 comments · Fixed by #14117
Labels
release/blocker Indicate the PR or issue that should block the release until it gets resolved type/bug The PR fixed a bug or issue reported a bug

Comments

@nicoloboschi
Copy link
Contributor

nicoloboschi commented Feb 3, 2022

Describe the bug
After #12818, a client (built starting from the master branch) get not found while trying to create a topic, if the server doesn't support admin v3 topic endpoint (currently only the master branch has this endpoint).

To Reproduce

  • Run Pulsar standalone 2.9.x (or prior)
  • Run command (from master branch) bin/pulsar-admin topics create-partitioned-topic test -p 1
    You get:
2022-02-03T12:26:16,790+0000 [AsyncHttpClient-7-1] WARN  org.apache.pulsar.client.admin.internal.BaseResource - [http://172.17.0.2:8080/admin/v3/persistent/public/default/test/partitions?createLocalTopicOnly=false] Failed to perform http put request: javax.ws.rs.NotFoundException: HTTP 404 Not Found
HTTP 404 Not Found

Reason: HTTP 404 Not Found

Expected behavior
The client should be send the request to v2 rest api endpoint

@nicoloboschi nicoloboschi added the type/bug The PR fixed a bug or issue reported a bug label Feb 3, 2022
@nicoloboschi nicoloboschi changed the title Can't create topics after #12818 Can't create topics after #12818 using Pulsar master client and 2.9.x broker (v3/persistent NOT FOUND) Feb 3, 2022
@eolivelli
Copy link
Contributor

@nicoloboschi good catch!

We should discuss about this on dev@ and we must fix this somehow

@eolivelli
Copy link
Contributor

So this comment is not correct
#12818 (comment)

@eolivelli eolivelli added the release/blocker Indicate the PR or issue that should block the release until it gets resolved label Feb 3, 2022
@nicoloboschi nicoloboschi changed the title Can't create topics after #12818 using Pulsar master client and 2.9.x broker (v3/persistent NOT FOUND) Can't create topics after #12818 using Pulsar master client and 2.9.x (or prior) broker (v3/persistent NOT FOUND) Feb 3, 2022
@lhotari
Copy link
Member

lhotari commented Feb 4, 2022

This was discussed in the Pulsar community meeting. Some notes from the discussion:

  • PR PIP 110: Support Topic metadata - PART-1 create topic with properties #12818 changes should be reverted so that API could be kept backwards compatible.
  • It’s possible to have the current v2 API for creating persistent topics to handle multiple different payload types
    The challenge seems to be that in the old method, the number of partitions was passed in the payload as a single integer without having a JSON object at all. This makes it harder to extend the existing API, but doesn’t make it impossible.
  • One possible solution is based on request’s “Content Type”
    Content-Type: application/vnd.some-custom-identifier+json on request, @Consumes("application/vnd.some-custom-identifier+json") on method that handles the new payload type. It also needs an annotation that the newer payload uses the custom mime type.

@lhotari
Copy link
Member

lhotari commented Feb 4, 2022

The custom content type could be set here:

if (properties != null) {
PartitionedTopicMetadata metadata = new PartitionedTopicMetadata(numPartitions, properties);
entity = Entity.entity(metadata, MediaType.APPLICATION_JSON);
} else {
entity = Entity.entity(numPartitions, MediaType.APPLICATION_JSON);
}

Would be MediaType.valueOf("application/vnd.some-custom-identifier+json")

@eolivelli
Copy link
Contributor

@lhotari would you please the PR with the revert?
This way we will unblock downstream consumers of the master branch

@lhotari
Copy link
Member

lhotari commented Feb 4, 2022

@lhotari would you please the PR with the revert? This way we will unblock downstream consumers of the master branch

It might be simpler to create a PR to fix the issue. I can do that.

@lhotari
Copy link
Member

lhotari commented Feb 4, 2022

I pushed #14117 as a fix that uses the custom JSON media type solution.

@lhotari
Copy link
Member

lhotari commented Feb 4, 2022

I noticed that this is needed to fix the issue too: 02a3201

That alone would fix the issue. However I think it's better to not add a new API version in this way since it's not even consistent. For all other methods, the support for PIP-110 was added directly to the existing v2 API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/blocker Indicate the PR or issue that should block the release until it gets resolved type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants