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.

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

[SPARK-12489][Core][SQL][MLib]Fix minor issues found by FindBugs #10440

Closed
wants to merge 1 commit into from
Closed

[SPARK-12489][Core][SQL][MLib]Fix minor issues found by FindBugs #10440

wants to merge 1 commit into from

Conversation

zsxwing
Copy link
Member

@zsxwing zsxwing commented Dec 22, 2015

Include the following changes:

  1. Close java.sql.Statement
  2. Fix incorrect asInstanceOf.
  3. Remove unnecessary synchronized and ReentrantLock.

zsxwing
zsxwing reviewed Dec 22, 2015
View reviewed changes
launcher/src/main/java/org/apache/spark/launcher/LauncherServer.java
@@ -293,9 +293,7 @@ private String createSecret() {
protected void handle(Message msg) throws IOException {
try {
if (msg instanceof Hello) {
synchronized (timeout) {
Copy link
Member Author

Choose a reason for hiding this comment

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

timeout.cancel already uses a lock internally.

@SparkQA
Copy link

SparkQA commented Dec 23, 2015

Test build #48215 has finished for PR 10440 at commit 6861fdd.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@jkbradley
Copy link
Member

ML changes look good to me. Thanks!

@zsxwing
Copy link
Member Author

zsxwing commented Dec 28, 2015

@andrewor14 could you take a look at this pr? Thanks!

@andrewor14
Copy link
Contributor

Looks good.

@andrewor14
Copy link
Contributor

Maybe @yhuai or @liancheng should take a quick look at the SQL changes.

@yhuai
Copy link
Contributor

yhuai commented Dec 28, 2015

sql changes look good.

@zsxwing
Copy link
Member Author

zsxwing commented Dec 28, 2015

Thanks. Merging to master and 1.6

asfgit pushed a commit that referenced this pull request Dec 28, 2015
@zsxwing
[SPARK-12489][CORE][SQL][MLIB] Fix minor issues found by FindBugs
fd20248 
Include the following changes:

1. Close `java.sql.Statement`
2. Fix incorrect `asInstanceOf`.
3. Remove unnecessary `synchronized` and `ReentrantLock`.

Author: Shixiong Zhu <[email protected]>

Closes #10440 from zsxwing/findbugs.

(cherry picked from commit 710b411)
Signed-off-by: Shixiong Zhu <[email protected]>
@asfgit asfgit closed this in 710b411 Dec 28, 2015
@zsxwing zsxwing deleted the findbugs branch December 28, 2015 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zsxwing @SparkQA @jkbradley @andrewor14 @yhuai