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

Error displaying schemas with id > 1000 #1632

Closed
omrazCZ opened this issue Dec 12, 2023 · 3 comments · Fixed by #1654
Closed

Error displaying schemas with id > 1000 #1632

omrazCZ opened this issue Dec 12, 2023 · 3 comments · Fixed by #1654

Comments

@omrazCZ
Copy link

omrazCZ commented Dec 12, 2023

Hi,

We are using AKHQ 0.23.0 to display our topics and schemas (stored in Apicurio 2.4).

When clicking on the schema ID associated with the message, the schema should be displayed. This seems to work fine for schema IDs under 1000, but for schemas with ID over 1000 we get a 504 Gateway Time-out.

Is 1000 a hard limit?

@AlexisSouquiere
Copy link
Collaborator

I experienced the same issue on the latest AKHQ versions too. I started to have a look months ago, it seems that we iterate over all the registry to find a subject that match the schema ID so it leads to a timeout issue on large registries. Here is the fonction called that produces the timeout issue:

public Optional<Schema> getById(String clusterId, Integer id) throws IOException, RestClientException, ExecutionException, InterruptedException {
for (String subject: this.all(clusterId, Optional.empty(), List.of())) {
for (Schema version: this.getAllVersions(clusterId, subject)) {
if (version.getId().equals(id)) {
return Optional.of(version);
}
}
}
return Optional.empty();
}

rovan pushed a commit to rovan/akhq that referenced this issue Dec 14, 2023
These changes supposed to fix the problem presented.
But there might be a compatibility issue due to use of a relatively new
Schema Registry REST method
https://docs.confluent.io/platform/current/schema-registry/develop/api.html#get--schemas-ids-int-%20id-versions
@rovan
Copy link

rovan commented Dec 14, 2023

I experienced the same issue on the latest AKHQ versions too. I started to have a look months ago, it seems that we iterate over all the registry to find a subject that match the schema ID so it leads to a timeout issue on large registries. Here is the fonction called that produces the timeout issue:

public Optional<Schema> getById(String clusterId, Integer id) throws IOException, RestClientException, ExecutionException, InterruptedException {
for (String subject: this.all(clusterId, Optional.empty(), List.of())) {
for (Schema version: this.getAllVersions(clusterId, subject)) {
if (version.getId().equals(id)) {
return Optional.of(version);
}
}
}
return Optional.empty();
}

Hello.
It looks like access time could be improved by using another REST method.
I made some changes that should fix the issue, but I'm not good enough at running AKHQ from the baseline, so I can't check it on a sufficiently large Schema Registry.

@AlexisSouquiere
Copy link
Collaborator

AlexisSouquiere commented Jan 16, 2024

Indeed, calling getAllVersionsById on the registry client like you did will return only the subjects using this version of the schema. The problem I'm seeing is that, like with the current implementation, we may won't get the right subject but another subject using the same schema.

If the schema used by your topic is used by other topics, the getAllVersionsById call will return something like this:

[{"subject":"topic1-value","version":1},{"subject":"topic2-value","version":2},{"subject":"topic3-value","version":1}]

If you click on the schema ID from a topic2 message, it will redirect you to the topic1-value subject because the test based on the id is actually the schema id, which is the same for "topic1-value", "topic2-value" and "topic3-value". Basically, it will always return the 1st one.
Based on that I think I'll propose to improve api/{cluster}/schema/id/{id} to take the topic name too to do the subject comparison on it and return the right one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants