keith-turner commented on a change in pull request #2320:
URL: https://github.com/apache/accumulo/pull/2320#discussion_r752618284



##########
File path: 
server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
##########
@@ -791,6 +798,50 @@ public void run() {
       }
     }, 0, 5000, TimeUnit.MILLISECONDS);
 
+    // Periodically check that metadata of tablets matches what is held in 
memory
+    
ThreadPools.createGeneralScheduledExecutorService(aconf).scheduleWithFixedDelay(()
 -> {
+      final SortedMap<KeyExtent,Tablet> onlineTabletsSnapshot = 
onlineTablets.snapshot();
+
+      final SortedSet<KeyExtent> userTablets = new TreeSet<>();
+      final SortedSet<KeyExtent> nonUserTablets = new TreeSet<>();
+
+      // Create subsets of tablets based on DataLevel: one set who's DataLevel 
is USER and another
+      // containing the remaining tablets (those who's DataLevel is ROOT or 
METADATA).
+      // This needs to happen so we can use .readTablets() on the 
DataLevel.USER tablets in order
+      // to reduce RPCs.
+      // TODO: Push this partitioning, based on DataLevel, to ample.
+      onlineTabletsSnapshot.forEach((k, v) -> {
+        if (Ample.DataLevel.of(k.tableId()) == Ample.DataLevel.USER) {
+          userTablets.add(k);
+        } else {
+          nonUserTablets.add(k);
+        }
+      });
+
+      List<TabletMetadata> tmdList;
+
+      // gather metadata for all tablets with DataLevel.USER using 
readTablets()
+      try (TabletsMetadata tabletsMetadata = 
getContext().getAmple().readTablets()
+          .forTablets(userTablets).fetch(FILES, LOGS, ECOMP, 
PREV_ROW).build()) {
+        tmdList = new 
ArrayList<>(IteratorUtils.toList(tabletsMetadata.iterator()));
+      }
+
+      // gather metadata for all tablets with DataLevel.ROOT or METADATA using 
readTablet()
+      nonUserTablets.forEach(extent -> {
+        TabletMetadata tabletMetadata =
+            getContext().getAmple().readTablet(extent, FILES, LOGS, ECOMP, 
PREV_ROW);
+        tmdList.add(tabletMetadata);
+      });
+
+      // for each tablet, compare its metadata to what is held in memory
+      for (TabletMetadata tabletMetadata : tmdList) {

Review comment:
       > Do you think it would suffice to check that each KeyExtent in 
onlineTabletsSnapshot is present within one of the returned TabletMetadata?
   
   That is the gist of what I was thinking about.   Thinking about it a bit 
more, maybe we don't want to worry about this case.  Under normal conditions a 
tablet could be deleted after we have grabbed the snapshot of online tablets.  
So if we did bother detecting it, we would not want to take any action that may 
cause alarm.  So if we did detect would only want to log a debug or something.  
If we are not going to worry about it or attempt to detect it, then looping 
over tmdList as you have is fine.  If there is an extent missing from tmdList 
that is in the snapshot, nothing will be done.
   
   




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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to