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
Fix the issue with "pinot-pulsar" module (potentially library conflicts) #7270
Comments
Currently, pinot-pulsar module is reported to have some issues on runtime. This PR removes the pinot-pulsar module from the binary release. apache#7270
Currently, pinot-pulsar module is reported to have some issues on runtime. This PR removes the pinot-pulsar module from the binary release. #7270
Currently, pinot-pulsar module is reported to have some issues on runtime. This PR removes the pinot-pulsar module from the binary release. #7270
Hello, |
We should probably shade some of the conflicting libraries |
Hi, I had a look at this issue, as I'm currently investigating pulsar and pinot integration for my current company. So far, it does not look like an issue with false shaded libraries. As one can see from the stacktraces, the errors occur in libraries that are already shaded by the pulsar client lib. I looked into the second
As you can see from the comment the assumption about the structure of a MessageId is made. Unfortunately the passed String is only the |
@KKcorps can you help here? |
Taking it up. |
thanks, I linked the changes that I made for it to work for me as a draft PR, in case it helps you. |
@aleksdikanski I remember implementing it this way because I don't want to make any assumptions regarding message-id format in the pinot code. The pulsar team can change the message-id format but our code should still work as long as pulsar lib is handling the new format correctly. I remember testing this out as well but your concern is valid. I am looking for another solution if that doesn't work will go with your PR one. |
@snleee @aleksdikanski can you tell me how to reproduce this issue? I feel like this can also be related to offset at the startup of the client. If might be throwing error in case of empty string. |
Sure @KKcorps , I basically followed the guides on the pulsar and pinot websites:
funny enough I tested this today and actually got another error that is different from the two mentioned above:
it also related to the MessageId parsing and was also fixed with my implementation |
Can you also mentioned the Pulsar version you are testing this on? Pinot version seems to be 0.9.1 from the exception logs. |
I did give the pulsar docker images, which uses the latest version 2.8.1, |
I also dig a bit yesterday and saw, that the start offset for a segment is fetched from Zookeeper. So I looked at the entry (PinotCluster / PROPERTYSTORE / SEGMENTS / / ) and saw, that the offset is set to the |
Yeah, I think the string offsets are borrowed from the Kafka implementation. We might need to make this more generic. That's also another reason I don't want to split the message id on |
@aleksdikanski In which file does this exception occur? Is it in pinot-all.log or pinot-quickstart.log or during creation of the table itself when running the pinot-admin command? |
Hi, I used the log output of the docker containers, so actually no log files were created. the error occurred in the logs of the pinot-server container at the time the first message was read from the topic. so I guess it is fair to say, it occurred when the first segment was created. |
@aleksdikanski #7947 can you try this and see if it is working on your end. It is working on my end. There was also a corrupt I am also noticing some issue with using the auto.offset.reset to largest. It works fine when using |
@KKcorps thanks I tested your changes and I don't get any errors regarding the MessageId anymore.
Must have been a copy paste error, did not have a problem with the original file. |
Hi @KKcorps are there any updates on this? Can I support in any way? |
Hi, The PR for the fix is already merged. Is there any other issue with code will be happy to help. |
@KKcorps You mentioned something not working with the
I'm not using the |
My bad. I need to get on this issue again. I remember testing it out thoroughly and it seemed to be a problem with my test case rather than the plugin itself. I will update you in next 24 hrs. |
https://github.com/apache/pinot/pull/8017/files |
@KKcorps any updates on this? is there anything to help here? |
Hi The PR was upgraded from draft status to ready for review. Still waiting to be merged |
Hi @KKcorps, could you also revert the change in https://github.com/apache/pinot/pull/7272/files in your PR? Otherwise the issue will be fixed but pulsar plugin will not be included in the next pinot release. |
Apache Pulsar connector has been added from #7026
However, it currently is facing some issues on runtime (potentially dependency conflicts). We need to fix the conflicts to make the connector work correctly.
The text was updated successfully, but these errors were encountered: