[GitHub] [hadoop] leosunli commented on a change in pull request #1885: HDFS-13639. SlotReleaser is not fast enough

[GitBox]

leosunli commented on a change in pull request #1885:
URL: https://github.com/apache/hadoop/pull/1885#discussion_r426277924



##########
File path: 
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/shortcircuit/ShortCircuitCache.java
##########
@@ -181,25 +182,49 @@ public long getRateInMs() {
 
     @Override
     public void run() {
+      if (slot == null) {
+        return;
+      }
       LOG.trace("{}: about to release {}", ShortCircuitCache.this, slot);
       final DfsClientShm shm = (DfsClientShm)slot.getShm();
       final DomainSocket shmSock = shm.getPeer().getDomainSocket();
       final String path = shmSock.getPath();
+      DataOutputStream out = null;
       boolean success = false;
-      try (DomainSocket sock = DomainSocket.connect(path);
-           DataOutputStream out = new DataOutputStream(
-               new BufferedOutputStream(sock.getOutputStream()))) {
-        new Sender(out).releaseShortCircuitFds(slot.getSlotId());
-        DataInputStream in = new DataInputStream(sock.getInputStream());
-        ReleaseShortCircuitAccessResponseProto resp =
-            ReleaseShortCircuitAccessResponseProto.parseFrom(
-                PBHelperClient.vintPrefixed(in));
-        if (resp.getStatus() != Status.SUCCESS) {
-          String error = resp.hasError() ? resp.getError() : "(unknown)";
-          throw new IOException(resp.getStatus().toString() + ": " + error);
+      int retries = 2;
+      try {
+        while (retries > 0) {
+          try {
+            if (domainSocket == null || !domainSocket.isOpen()) {
+              // we are running in single thread mode, no protection needed for

Review comment:
       yeah, the SlotReleaser is running in single thread as follow code.
    
ShortCircuitCache#uref->replica.close()->cache.scheduleSlotReleaser(slot)->releaserExecutor.execute(new
 SlotReleaser(slot))-> SlotReleaser#run {
   ...
   try {
               if (domainSocket == null || !domainSocket.isOpen()) {
                 // we are running in single thread mode, no protection needed 
for
                 // domainSocket
                 domainSocket = DomainSocket.connect(path);
               }
   ...
   }
   Since ShortCircuitCache#uref run in lock，this code is run serially in a core 
thread of a thread pool.




----------------------------------------------------------------
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:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org