https://venicedb.org logo
Join Slack
Powered by
Hi everyone, We have been trying to integrate the ...
# general
m
Hi everyone, We have been trying to integrate the spark push job into our workflow. First of all thank you for the awesome work. However we are facing some issues with the usage of scala 2.12 used in the dependancies as we are using 2.13. Is there a specific reason to be on 2.12 ? I see that there is a dependancy on the linkdin fork of kafka which is only on 2.12 which might be the problem. It won't be possible to integrate the push job or the relevant spark writers to our code as 2.12 and 2.13 are incompatible. It should be easy to change the spark dependancies to 2.13 but the custom kafka library is going to be a problem. So is there a way for us to get around this or have a 2.13 build as well.
f
Hmm… thanks for flagging… in theory, the 2.12 we get from Kafka should only be pulled into the integration tests, and none of the main modules should depend on it… if there is such dependency, we ought to exclude it.
Would you mind opening a GH Issue for this?
BTW… in my latest commit, I have sneaked in a change to remove the dependency on 
kafka_2.12
from the main modules (including VPJ)… but I have not tried yet bumping the spark dependency to 2.13… IDK if that will work, but we can try.
FYI, I took a stab at bumping scala to 2.13… I hit various issues, solved some of them, but ultimately it seems to be a dead end because of the reason you mentioned: the LI fork of Kafka is only published for 2.12… so there seems to be two options: 1. Get the LI Kafka team to cross-publish both 2.12 and 2.13 (I’ll talk to them, but not sure there is appetite for that…), or… 2. Migrate the open source Venice project to Apache Kafka, and internally we’ll swap that for the LI Kafka fork instead. We’ll evaluate these options and report back…
✅ 1
@Sushant Mane, what do you think about option (2) above? Since we have pluggability of the pub sub provider now, in theory it should not be too hard to pull off, right?
👀 1
s
Yeah I think option 2 is easier and faster. Ideally, it should be just drop in replacement.
f
Ideally 😄
😅 1
s
AFAIK we do not really depend on anything LI specific, for pub-sub clients.
Let me try it out 😅
f
I tried a few things here… https://github.com/FelixGV/venice/tree/kafka_3_0_upgrade … though it’s not quite what Moditha wants.
🙌 1
gratitude thank you 1
s
I'm not sure if I follow. We can also consider moving pub-sub api implementation for Kafka to a separate module. And make code completey free of Kafka deps. (I think we're super close to this. There are just few more classes where cleanup is required. )
f
For some reason I thought the kafka bump would help me get access to newer scala versions as well, though that is not the case. So the Kafka bump is orthogonal to this, and unnecessary.
That being said… even when staying on Kafka 2.4.1, there are incompatibilities between the LI fork and the Apache one… for example 
org.apache.kafka.common.protocol.SecurityProtocol
apparently does not exist in Apache Kafka 🤔 ….
s
f
They changed the package name to 
org.apache.kafka.common.security.auth.SecurityProtocol
😓 😅
😅 1
We will probably need to stop using that enum entirely… have our own enum for it instead… and map it to the actual class that exists in either fork inside some adapter layer.
Another incompatiblity: producer flush does not support a timeout in AK… only in LIK.
These are minor annoyances… but would need to be dealt with to make option (2) work.
s
Yeah agree. Are you working on this now? We do have plans to get rid of remaining Kafka references from the code (will be done during vpj and dvc migration to non-Kakfka clients)
f
For now I’m just trying to get a branch to compile with AK APIs… but I don’t think I’ll have the bandwidth to make it modular everywhere and ensure that the internal shims are updated accordingly, etc.
So if I succeed at all it would provide just a reference branch of where all the touch points are…
s
Oh ok. Thanks!
f
I’m hitting some very weird issues… the integration test module refuses to take up the Apache Kafka dependency, but all other deps are still coming through fine, and other modules are also taking up the AK dep fine… There must be some subtle build issue, but I’ve never seen that before.
s
But E2E with your version bump are passing?
f
Well that’s good I guess… though that is still with the LI fork
This is how far I’ve gotten on the Apache fork: https://github.com/FelixGV/venice/tree/apache_kafka_migration
s
I think you're still using old package name: import org.apache.kafka.common.protocol.SecurityProtocol; 380
f
A bunch of classes not found, including some very basic ones that definitely didn’t change package… e.g.:
Copy code
/home/runner/work/venice/venice/internal/venice-test-common/src/integrationTest/java/com/linkedin/venice/hadoop/input/kafka/TestKafkaInputFormat.java:26: error: package org.apache.kafka.common does not exist
import org.apache.kafka.common.TopicPartition;
                              ^
There’s something wrong with the integ test module specifically… I don’t understand.
Ah, damn it 😓 … the build.gradle file for integ tests has this 😅 …
Copy code
implementation {
    exclude group: 'org.apache.kafka'
🫣 1
😂 1
And another one, to see if the scala bump can be made to work… https://github.com/FelixGV/venice/tree/apache_kafka_and_scala_2.13
gratitude thank you 2
m
Sorry I'm in CET. I have created the following issue https://github.com/linkedin/venice/issues/998 Thank you very much for pushing this its great. In the meantime I have made a bugfix on the spark code that prevented us from reading from S3 https://github.com/linkedin/venice/pull/999
🚀 1
f
Awesome! I merged it! Thanks a lot!
I opened PR 1000 (!) to do another step in this direction: https://github.com/linkedin/venice/pull/1000
🎉 1
gratitude thank you 1
Hi @Moditha Hewasinghage, I want to give an update on this. These are the steps I envision to get you a Scala 2.13-based VPJ: 1. I’ve taken a bit of a detour to cleanly get rid of our usage of the “producer flush with timeout” API that we use but which exists only in the LI fork of Kafka. This change (PR 1014) should be about to get merged. 2. After that, I’ll re-open my PR 1000, above, rebase it on the above change, and we’ll get that merged. Should hopefully not take too long… 3. Then, finally, I’ll have to tweak our build so that we cross-compile VPJ against Scala 2.12 and 2.13, because we still use 2.12 in our own deployment… There are a few options here, and I’ve not decided which one to use yet. If anyone has any suggestions, I’m all ears.
PR 1000 is now rebased, so we may be able to finish step 2 soon…
m
Awesome thanks for the update
f
FYI, PR 1000 is in and of itself ready to merge, but we're making some changes to our internal build to cope with this change in open source. Should be ready soon, hopefully, then we can merge the open source PR.
🙌 1
4 Views
Open in Slack
PreviousNext
Powered by