[kudu-CR] java: fix minor synchronization issues exposed by error-prone
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10199 ) Change subject: java: fix minor synchronization issues exposed by error-prone .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c737f59928f393883d198872419e8832dfff006 Gerrit-Change-Number: 10199 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 30 Apr 2018 20:56:48 + Gerrit-HasComments: No
[kudu-CR] java: fix minor synchronization issues exposed by error-prone
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10199 ) Change subject: java: fix minor synchronization issues exposed by error-prone .. java: fix minor synchronization issues exposed by error-prone * Unsynchronized access to 'sessions' in AsyncKuduClient.closeAllSessions(). This isn't likely to cause problems since it only happens on 'close' and would only race if a new session was concurrently started. * Add missing GuardedBy annotations in a few spots (non-functional) * Fix synchronization in TableLocationsCache.toString() to properly acquire the lock. Unlikely to cause problems in practice since we only call toString() on this in some rare trace messages (if that). * Fix synchronization in FakeDNS (test-only code) Change-Id: I1c737f59928f393883d198872419e8832dfff006 Reviewed-on: http://gerrit.cloudera.org:8080/10199 Reviewed-by: Grant Henke Tested-by: Todd Lipcon --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/FakeDNS.java 4 files changed, 16 insertions(+), 5 deletions(-) Approvals: Grant Henke: Looks good to me, approved Todd Lipcon: Verified -- To view, visit http://gerrit.cloudera.org:8080/10199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I1c737f59928f393883d198872419e8832dfff006 Gerrit-Change-Number: 10199 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] java: fix minor synchronization issues exposed by error-prone
Todd Lipcon has removed a vote on this change. Change subject: java: fix minor synchronization issues exposed by error-prone .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/10199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I1c737f59928f393883d198872419e8832dfff006 Gerrit-Change-Number: 10199 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] java: fix minor synchronization issues exposed by error-prone
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10199 ) Change subject: java: fix minor synchronization issues exposed by error-prone .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c737f59928f393883d198872419e8832dfff006 Gerrit-Change-Number: 10199 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 30 Apr 2018 19:57:57 + Gerrit-HasComments: No
[kudu-CR] java: fix minor synchronization issues exposed by error-prone
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10199 ) Change subject: java: fix minor synchronization issues exposed by error-prone .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/10199/1/java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java File java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java: http://gerrit.cloudera.org:8080/#/c/10199/1/java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java@237 PS1, Line 237: rwl.readLock(); > woops, yes! I'm surprised error-prone doesn't have a check for this pattern Done -- To view, visit http://gerrit.cloudera.org:8080/10199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c737f59928f393883d198872419e8832dfff006 Gerrit-Change-Number: 10199 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 30 Apr 2018 19:23:46 + Gerrit-HasComments: Yes
[kudu-CR] java: fix minor synchronization issues exposed by error-prone
Hello Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10199 to look at the new patch set (#3). Change subject: java: fix minor synchronization issues exposed by error-prone .. java: fix minor synchronization issues exposed by error-prone * Unsynchronized access to 'sessions' in AsyncKuduClient.closeAllSessions(). This isn't likely to cause problems since it only happens on 'close' and would only race if a new session was concurrently started. * Add missing GuardedBy annotations in a few spots (non-functional) * Fix synchronization in TableLocationsCache.toString() to properly acquire the lock. Unlikely to cause problems in practice since we only call toString() on this in some rare trace messages (if that). * Fix synchronization in FakeDNS (test-only code) Change-Id: I1c737f59928f393883d198872419e8832dfff006 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/FakeDNS.java 4 files changed, 16 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/10199/3 -- To view, visit http://gerrit.cloudera.org:8080/10199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1c737f59928f393883d198872419e8832dfff006 Gerrit-Change-Number: 10199 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] java: fix minor synchronization issues exposed by error-prone
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10199 ) Change subject: java: fix minor synchronization issues exposed by error-prone .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/10199/1/java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java File java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java: http://gerrit.cloudera.org:8080/#/c/10199/1/java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java@237 PS1, Line 237: rwl.readLock(); > Should this be .lock() and .unlock() below? woops, yes! I'm surprised error-prone doesn't have a check for this pattern. -- To view, visit http://gerrit.cloudera.org:8080/10199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c737f59928f393883d198872419e8832dfff006 Gerrit-Change-Number: 10199 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 26 Apr 2018 22:20:30 + Gerrit-HasComments: Yes
[kudu-CR] java: fix minor synchronization issues exposed by error-prone
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10199 ) Change subject: java: fix minor synchronization issues exposed by error-prone .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/10199/1/java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java File java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java: http://gerrit.cloudera.org:8080/#/c/10199/1/java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java@237 PS1, Line 237: rwl.readLock(); Should this be .lock() and .unlock() below? -- To view, visit http://gerrit.cloudera.org:8080/10199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c737f59928f393883d198872419e8832dfff006 Gerrit-Change-Number: 10199 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Thu, 26 Apr 2018 14:41:27 + Gerrit-HasComments: Yes
[kudu-CR] java: fix minor synchronization issues exposed by error-prone
Hello Grant Henke, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/10199 to review the following change. Change subject: java: fix minor synchronization issues exposed by error-prone .. java: fix minor synchronization issues exposed by error-prone * Unsynchronized access to 'sessions' in AsyncKuduClient.closeAllSessions(). This isn't likely to cause problems since it only happens on 'close' and would only race if a new session was concurrently started. * Add missing GuardedBy annotations in a few spots (non-functional) * Fix synchronization in TableLocationsCache.toString() to properly acquire the lock. Unlikely to cause problems in practice since we only call toString() on this in some rare trace messages (if that). * Fix synchronization in FakeDNS (test-only code) Change-Id: I1c737f59928f393883d198872419e8832dfff006 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/FakeDNS.java 4 files changed, 16 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/10199/1 -- To view, visit http://gerrit.cloudera.org:8080/10199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1c737f59928f393883d198872419e8832dfff006 Gerrit-Change-Number: 10199 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Grant Henke
