You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/04/14 13:23:00 UTC

[GitHub] [accumulo] keith-turner commented on a change in pull request #1584: Moved ThriftClientHandler out of TabletServer for #1581

keith-turner commented on a change in pull request #1584: Moved ThriftClientHandler out of TabletServer for #1581
URL: https://github.com/apache/accumulo/pull/1584#discussion_r408128575
 
 

 ##########
 File path: server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
 ##########
 @@ -2278,301 +529,13 @@ public void enqueueMasterMessage(MasterMessage m) {
     masterMessages.addLast(m);
   }
 
-  private class UnloadTabletHandler implements Runnable {
-    private final KeyExtent extent;
-    private final TUnloadTabletGoal goalState;
-    private final long requestTimeSkew;
-
-    public UnloadTabletHandler(KeyExtent extent, TUnloadTabletGoal goalState, long requestTime) {
-      this.extent = extent;
-      this.goalState = goalState;
-      this.requestTimeSkew = requestTime - MILLISECONDS.convert(System.nanoTime(), NANOSECONDS);
-    }
-
-    @Override
-    public void run() {
-
-      Tablet t = null;
-
-      synchronized (unopenedTablets) {
-        if (unopenedTablets.contains(extent)) {
-          unopenedTablets.remove(extent);
-          // enqueueMasterMessage(new TabletUnloadedMessage(extent));
-          return;
-        }
-      }
-      synchronized (openingTablets) {
-        while (openingTablets.contains(extent)) {
-          try {
-            openingTablets.wait();
-          } catch (InterruptedException e) {}
-        }
-      }
-      synchronized (onlineTablets) {
-        if (onlineTablets.snapshot().containsKey(extent)) {
-          t = onlineTablets.snapshot().get(extent);
-        }
-      }
-
-      if (t == null) {
-        // Tablet has probably been recently unloaded: repeated master
-        // unload request is crossing the successful unloaded message
-        if (!recentlyUnloadedCache.containsKey(extent)) {
-          log.info("told to unload tablet that was not being served {}", extent);
-          enqueueMasterMessage(
-              new TabletStatusMessage(TabletLoadState.UNLOAD_FAILURE_NOT_SERVING, extent));
-        }
-        return;
-      }
-
-      try {
-        t.close(!goalState.equals(TUnloadTabletGoal.DELETED));
-      } catch (Throwable e) {
-
-        if ((t.isClosing() || t.isClosed()) && e instanceof IllegalStateException) {
-          log.debug("Failed to unload tablet {}... it was already closing or closed : {}", extent,
-              e.getMessage());
-        } else {
-          log.error("Failed to close tablet {}... Aborting migration", extent, e);
-          enqueueMasterMessage(new TabletStatusMessage(TabletLoadState.UNLOAD_ERROR, extent));
-        }
-        return;
-      }
-
-      // stop serving tablet - client will get not serving tablet
-      // exceptions
-      recentlyUnloadedCache.put(extent, System.currentTimeMillis());
-      onlineTablets.remove(extent);
-
-      try {
-        TServerInstance instance = new TServerInstance(clientAddress, getLock().getSessionId());
-        TabletLocationState tls = null;
-        try {
-          tls = new TabletLocationState(extent, null, instance, null, null, null, false);
-        } catch (BadLocationStateException e) {
-          log.error("Unexpected error", e);
-        }
-        if (!goalState.equals(TUnloadTabletGoal.SUSPENDED) || extent.isRootTablet()
-            || (extent.isMeta()
-                && !getConfiguration().getBoolean(Property.MASTER_METADATA_SUSPENDABLE))) {
-          TabletStateStore.unassign(getContext(), tls, null);
-        } else {
-          TabletStateStore.suspend(getContext(), tls, null,
-              requestTimeSkew + MILLISECONDS.convert(System.nanoTime(), NANOSECONDS));
-        }
-      } catch (DistributedStoreException ex) {
-        log.warn("Unable to update storage", ex);
-      } catch (KeeperException e) {
-        log.warn("Unable determine our zookeeper session information", e);
-      } catch (InterruptedException e) {
-        log.warn("Interrupted while getting our zookeeper session information", e);
-      }
-
-      // tell the master how it went
-      enqueueMasterMessage(new TabletStatusMessage(TabletLoadState.UNLOADED, extent));
-
-      // roll tablet stats over into tablet server's statsKeeper object as
-      // historical data
-      statsKeeper.saveMajorMinorTimes(t.getTabletStats());
-    }
-  }
-
-  protected class AssignmentHandler implements Runnable {
-    private final KeyExtent extent;
-    private final int retryAttempt;
-
-    public AssignmentHandler(KeyExtent extent) {
-      this(extent, 0);
-    }
-
-    public AssignmentHandler(KeyExtent extent, int retryAttempt) {
-      this.extent = extent;
-      this.retryAttempt = retryAttempt;
-    }
-
-    @Override
-    public void run() {
-      synchronized (unopenedTablets) {
-        synchronized (openingTablets) {
-          synchronized (onlineTablets) {
-            // nothing should be moving between sets, do a sanity
-            // check
-            Set<KeyExtent> unopenedOverlapping = KeyExtent.findOverlapping(extent, unopenedTablets);
-            Set<KeyExtent> openingOverlapping = KeyExtent.findOverlapping(extent, openingTablets);
-            Set<KeyExtent> onlineOverlapping =
-                KeyExtent.findOverlapping(extent, onlineTablets.snapshot());
-
-            if (openingOverlapping.contains(extent) || onlineOverlapping.contains(extent)) {
-              return;
-            }
-
-            if (!unopenedOverlapping.contains(extent)) {
-              log.info("assignment {} no longer in the unopened set", extent);
-              return;
-            }
-
-            if (unopenedOverlapping.size() != 1 || openingOverlapping.size() > 0
-                || onlineOverlapping.size() > 0) {
-              throw new IllegalStateException(
-                  "overlaps assigned " + extent + " " + !unopenedTablets.contains(extent) + " "
-                      + unopenedOverlapping + " " + openingOverlapping + " " + onlineOverlapping);
-            }
-          }
-
-          unopenedTablets.remove(extent);
-          openingTablets.add(extent);
-        }
-      }
-
-      // check Metadata table before accepting assignment
-      Text locationToOpen = null;
-      TabletMetadata tabletMetadata = null;
-      boolean canLoad = false;
-      try {
-        tabletMetadata = getContext().getAmple().readTablet(extent);
-
-        canLoad = checkTabletMetadata(extent, TabletServer.this.getTabletSession(), tabletMetadata);
-
-        if (canLoad && tabletMetadata.sawOldPrevEndRow()) {
-          KeyExtent fixedExtent =
-              MasterMetadataUtil.fixSplit(getContext(), tabletMetadata, getLock());
-
-          synchronized (openingTablets) {
-            openingTablets.remove(extent);
-            openingTablets.notifyAll();
-            // it expected that the new extent will overlap the old one... if it does not, it
-            // should not be added to unopenedTablets
-            if (!KeyExtent.findOverlapping(extent, new TreeSet<>(Arrays.asList(fixedExtent)))
-                .contains(fixedExtent)) {
-              throw new IllegalStateException(
-                  "Fixed split does not overlap " + extent + " " + fixedExtent);
-            }
-            unopenedTablets.add(fixedExtent);
-          }
-          // split was rolled back... try again
-          new AssignmentHandler(fixedExtent).run();
-          return;
-
-        }
-      } catch (Exception e) {
-        synchronized (openingTablets) {
-          openingTablets.remove(extent);
-          openingTablets.notifyAll();
-        }
-        log.warn("Failed to verify tablet " + extent, e);
-        enqueueMasterMessage(new TabletStatusMessage(TabletLoadState.LOAD_FAILURE, extent));
-        throw new RuntimeException(e);
-      }
-
-      if (!canLoad) {
-        log.debug("Reporting tablet {} assignment failure: unable to verify Tablet Information",
-            extent);
-        synchronized (openingTablets) {
-          openingTablets.remove(extent);
-          openingTablets.notifyAll();
-        }
-        enqueueMasterMessage(new TabletStatusMessage(TabletLoadState.LOAD_FAILURE, extent));
-        return;
-      }
-
-      Tablet tablet = null;
-      boolean successful = false;
-
-      try {
-        acquireRecoveryMemory(extent);
-
-        TabletResourceManager trm =
-            resourceManager.createTabletResourceManager(extent, getTableConfiguration(extent));
-        TabletData data = new TabletData(tabletMetadata);
-
-        tablet = new Tablet(TabletServer.this, extent, trm, data);
-        // If a minor compaction starts after a tablet opens, this indicates a log recovery
-        // occurred. This recovered data must be minor compacted.
-        // There are three reasons to wait for this minor compaction to finish before placing the
-        // tablet in online tablets.
-        //
-        // 1) The log recovery code does not handle data written to the tablet on multiple tablet
-        // servers.
-        // 2) The log recovery code does not block if memory is full. Therefore recovering lots of
-        // tablets that use a lot of memory could run out of memory.
-        // 3) The minor compaction finish event did not make it to the logs (the file will be in
-        // metadata, preventing replay of compacted data)... but do not
-        // want a majc to wipe the file out from metadata and then have another process failure...
-        // this could cause duplicate data to replay.
-        if (tablet.getNumEntriesInMemory() > 0
-            && !tablet.minorCompactNow(MinorCompactionReason.RECOVERY)) {
-          throw new RuntimeException("Minor compaction after recovery fails for " + extent);
-        }
-        Assignment assignment = new Assignment(extent, getTabletSession());
-        TabletStateStore.setLocation(getContext(), assignment);
-
-        synchronized (openingTablets) {
-          synchronized (onlineTablets) {
-            openingTablets.remove(extent);
-            onlineTablets.put(extent, tablet);
-            openingTablets.notifyAll();
-            recentlyUnloadedCache.remove(tablet.getExtent());
-          }
-        }
-        tablet = null; // release this reference
-        successful = true;
-      } catch (Throwable e) {
-        log.warn("exception trying to assign tablet {} {}", extent, locationToOpen, e);
-
-        if (e.getMessage() != null) {
-          log.warn("{}", e.getMessage());
-        }
-
-        TableId tableId = extent.getTableId();
-        ProblemReports.getInstance(getContext()).report(new ProblemReport(tableId, TABLET_LOAD,
-            extent.getUUID().toString(), getClientAddressString(), e));
-      } finally {
-        releaseRecoveryMemory(extent);
-      }
-
-      if (!successful) {
-        synchronized (unopenedTablets) {
-          synchronized (openingTablets) {
-            openingTablets.remove(extent);
-            unopenedTablets.add(extent);
-            openingTablets.notifyAll();
-          }
-        }
-        log.warn("failed to open tablet {} reporting failure to master", extent);
-        enqueueMasterMessage(new TabletStatusMessage(TabletLoadState.LOAD_FAILURE, extent));
-        long reschedule = Math.min((1L << Math.min(32, retryAttempt)) * 1000, 10 * 60 * 1000L);
-        log.warn(String.format("rescheduling tablet load in %.2f seconds", reschedule / 1000.));
-        SimpleTimer.getInstance(getConfiguration()).schedule(new TimerTask() {
-          @Override
-          public void run() {
-            log.info("adding tablet {} back to the assignment pool (retry {})", extent,
-                retryAttempt);
-            AssignmentHandler handler = new AssignmentHandler(extent, retryAttempt + 1);
-            if (extent.isMeta()) {
-              if (extent.isRootTablet()) {
-                new Daemon(new LoggingRunnable(log, handler), "Root tablet assignment retry")
-                    .start();
-              } else {
-                resourceManager.addMetaDataAssignment(extent, log, handler);
-              }
-            } else {
-              resourceManager.addAssignment(extent, log, handler);
-            }
-          }
-        }, reschedule);
-      } else {
-        enqueueMasterMessage(new TabletStatusMessage(TabletLoadState.LOADED, extent));
-      }
-    }
-  }
-
-  private void acquireRecoveryMemory(KeyExtent extent) {
+  protected void acquireRecoveryMemory(KeyExtent extent) {
 
 Review comment:
   ```suggestion
    void acquireRecoveryMemory(KeyExtent extent) {
   ```

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


With regards,
Apache Git Services