> On June 2, 2015, 12:23 a.m., Thejas Nair wrote: > > service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java, > > line 103 > > <https://reviews.apache.org/r/34776/diff/4/?file=975858#file975858line103> > > > > would it be possible to use a synchronized set instead ? That would be > > less error prone. > > > > > > Set<String> mySet = Collections.newSetFromMap(new > > ConcurrentHashMap<String, Boolean>()); > > > > I also see that we should be synchronizing on open/close in > > SessionManager and creation of new operations. But I think that is > > something for another jira since the primary goal of this one is not to fix > > issues when running multiple operations in a session. > > Sergey Shelukhin wrote: > close call take out all the elements in the set atomically (and clears > the set); normal set doesn't support such operation. I don't know how > necessary it is... > Given recent JDBC driver changes for multiple statements in one > connection this may be a good change to make to prevent bugs, now that it's > already done
I think the right way to do it is to synchronize on open/close vs creation of new operations in SessionManager. When the session is being closed, it should be within a lock and first thing it should do is to remove the Sessionhandle from the map so that no other operation can access it. It takes courage to delete old code, specially if they are doing some synchronization , I think it is better to do it the better way in a follow up jira. The JDBC driver changes does not suddenly enable use of multiple statements in one connection. That has always been there, that patch just adds some locks on it. So I think it is OK to do that as part of new jira. - Thejas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34776/#review86129 ----------------------------------------------------------- On June 2, 2015, 12:53 a.m., Sergey Shelukhin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34776/ > ----------------------------------------------------------- > > (Updated June 2, 2015, 12:53 a.m.) > > > Review request for hive. > > > Repository: hive-git > > > Description > ------- > > see jira > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java d733d71 > ql/src/java/org/apache/hadoop/hive/ql/Driver.java 5dac29f > ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezUtils.java 0edfc5d > ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 37b6d6f > service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java > 343c68e > > service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java > a29e5d1 > service/src/java/org/apache/hive/service/server/HiveServer2.java 58e8e49 > service/src/test/org/apache/hive/service/cli/CLIServiceTest.java b4d517f > > Diff: https://reviews.apache.org/r/34776/diff/ > > > Testing > ------- > > > Thanks, > > Sergey Shelukhin > >
