sodonnel commented on a change in pull request #1730:
URL: https://github.com/apache/ozone/pull/1730#discussion_r552958106



##########
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java
##########
@@ -516,22 +516,36 @@ public void importContainerData(InputStream input,
   @Override
   public void exportContainerData(OutputStream destination,
       ContainerPacker<KeyValueContainerData> packer) throws IOException {
-    // Closed/ Quasi closed containers are considered for replication by
-    // replication manager if they are under-replicated.
-    ContainerProtos.ContainerDataProto.State state =
-        getContainerData().getState();
-    if (!(state == ContainerProtos.ContainerDataProto.State.CLOSED ||
-        state == ContainerDataProto.State.QUASI_CLOSED)) {
-      throw new IllegalStateException(
-          "Only closed/quasi closed containers could be exported: " +
-              "Where as ContainerId="
-              + getContainerData().getContainerID() + " is in state " + state);
+    readLock();

Review comment:
       The exception here should be a rare occurrence, so we are almost always 
going to have to escalate the read lock into a write lock. I think it would be 
simpler to just take the write lock at the start and forget about the read lock.
   
   The part of the code under the read lock should be very fast, and the code 
under the write lock may be slow (compactDB) so I suspect we are saving very 
little by doing some work under the read lock. I think this lock is also only 
for this container (not for all containers on the DN) so contention should not 
be a big issue. 
   
   What do you think?




----------------------------------------------------------------
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]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to