Skip to content

Fix for using Broadcast variables in Databricks #766

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 58 commits into from
Nov 12, 2020
Merged

Fix for using Broadcast variables in Databricks #766

merged 58 commits into from
Nov 12, 2020

Conversation

Niharikadutta
Copy link
Collaborator

@Niharikadutta Niharikadutta commented Nov 4, 2020 •
edited
Loading

This PR changes how setupBroadcast is called for Databricks Environment, by passing javaSparkContext to it.
Change tested against Databricks Runtime version 6.6 and 7.3.

Fixes #754

Niharikadutta and others added 30 commits April 19, 2020 22:31
@Niharikadutta
Adding section for UDF serialization
03b7939
@Niharikadutta
removing guides from master
4ef693d
@Niharikadutta
Merge latest from master
81145ca
@Niharikadutta
merging latest from master
e4b81af
@Niharikadutta
Merge remote-tracking branch 'upstream/master'
4c32173
@Niharikadutta
Merge remote-tracking branch 'upstream/master'
4987a09
@Niharikadutta
Merge remote-tracking branch 'upstream/master'
ca9612e
@Niharikadutta
Merge remote-tracking branch 'upstream/master'
f581c86
@Niharikadutta
Merge remote-tracking branch 'upstream/master'
086b325
@Niharikadutta
Merge remote-tracking branch 'upstream/master'
2f72907
CountVectorizer
6bab996
moving private methods to bottom
e2a566b
changing wrap method
5f682a6
setting min version required
31371db
undoing csproj change
60eb82f
member doesnt need to be internal
ed36375
too many lines
c7baf72
removing whitespace change
d13303c
removing whitespace change
f5b477c
ionide
73db52b
@Niharikadutta
Merge remote-tracking branch 'upstream/master'
98f5e4d
@Niharikadutta
Merge remote-tracking branch 'upstream/master'
4c5d502
@GoEddie
Merge branch 'master' into ml/countvectorizer
a766146
@Niharikadutta
Merge branch 'ml/countvectorizer' of https://github.com/GoEddie/spark
ad6bced
@Niharikadutta
Revert "Merge branch 'master' into ml/countvectorizer"
8e1685c 
This reverts commit a766146, reversing
changes made to 73db52b.

Reverting countvectorizer changes
@Niharikadutta
Revert "Merge branch 'ml/countvectorizer' of https://github.com/GoEdd…
255515e 
…ie/spark"

This reverts commit ad6bced, reversing
changes made to 4c5d502.

reverting countvectorizer changes -2
@Niharikadutta
Merge remote-tracking branch 'upstream/master'
a44c882 
merging latest from master
@Niharikadutta
fixing merge errors
3c2c936
@Niharikadutta
removing ionid
88e834d
@Niharikadutta
Merge remote-tracking branch 'upstream/master'
59e7299 
Merging latest from master
@Niharikadutta
Copy link
Collaborator Author

@imback82 If this change makes sense, can we also add this to 1.0.1 milestone?

@imback82
Copy link
Contributor

imback82 commented Nov 6, 2020

Yes, can you update the description with the DB runtime you tested against?

@imback82 imback82 added this to the 1.0.1 milestone Nov 6, 2020
@imback82 imback82 added the enhancement New feature or request label Nov 6, 2020
@Niharikadutta Niharikadutta reopened this Nov 6, 2020
@imback82
Copy link
Contributor

imback82 commented Nov 6, 2020

@Niharikadutta Can you also test with DB runtime 7.X (Spark 3.0)?

@Niharikadutta
Copy link
Collaborator Author

@Niharikadutta Can you also test with DB runtime 7.X (Spark 3.0)?

@imback82 Tested with DB runtime 7.3 LTS as well.

@Niharikadutta
Copy link
Collaborator Author

Maven build is failing on Ubuntu with the following error:

[repo.maven.apache.org/151.101.188.215] failed: Connection timed out (Connection timed out)

@Niharikadutta
Copy link
Collaborator Author

@imback82 , @suhsteve , @elvaliuliuliu Could you please review this PR? Thanks!

@Niharikadutta Niharikadutta mentioned this pull request Nov 11, 2020
Closed
suhsteve
suhsteve previously approved these changes Nov 11, 2020
View reviewed changes
Copy link
Member

@suhsteve suhsteve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

imback82
imback82 reviewed Nov 11, 2020
View reviewed changes
Copy link
Contributor

@imback82 imback82 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as well (one nit comment), thanks @Niharikadutta!

@Niharikadutta @imback82
Update src/csharp/Microsoft.Spark/Broadcast.cs
d414755 
Co-authored-by: Terry Kim <yuminkim@gmail.com>
@Niharikadutta Niharikadutta reopened this Nov 12, 2020
@imback82 imback82 merged commit 87bdd6d into dotnet:master Nov 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@imback82 imback82 imback82 approved these changes

@suhsteve suhsteve suhsteve left review comments

Assignees

@Niharikadutta Niharikadutta

Labels
enhancement New feature or request
Projects
None yet
Milestone
1.1.0
Development

Successfully merging this pull request may close these issues.

[BUG]: Broadcast throws exception on Databricks on AWS.
4 participants
@Niharikadutta @imback82 @suhsteve @GoEddie