ashutosh-bapat commented on a change in pull request #669: HIVE-21864:
LlapBaseInputFormat#closeAll() throws ConcurrentModificationException
URL: https://github.com/apache/hive/pull/669#discussion_r292867947
##########
File path:
llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java
##########
@@ -345,8 +346,11 @@ public static void close(String handleId) throws
IOException {
*/
public static void closeAll() {
LOG.debug("Closing all handles");
- for (String handleId : connectionMap.keySet()) {
+ Iterator<String> handleIds = connectionMap.keySet().iterator();
+ String handleId = null;
+ while (handleIds.hasNext()) {
try {
+ handleId = handleIds.next();
Review comment:
Looking at https://docs.oracle.com/javase/7/docs/api/java/util/HashMap.html,
esp. the following para
> The iterators returned by all of this class's "collection view methods"
are fail-fast: if the map is structurally modified at any time after the
iterator is created, in any way except through the iterator's own remove
method, the iterator will throw a ConcurrentModificationException. Thus, in the
face of concurrent modification, the iterator fails quickly and cleanly, rather
than risking arbitrary, non-deterministic behavior at an undetermined time in
the future.
it looks like even the new code is prone to the same exception. Is the
synchronize block in close() protects the code from exception?. Ideally we
should be using iterator.remove() method to remove the element while iterating
over keySet().
I think, what we want to do here is
1. Write two close() methods, one accepting a handleId and other accepting
handleConnections. The first gets the handleConnections from handleId and calls
the second.
2. Modify closeAll() method to iterate over the connectionMap.keySet() using
an interator. For every element in the interator, get the handleConnections and
call close() method with those handleConnections. Now call remove() on the
iterator.
Does that look good?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
