[PR] Fix indentation in bulkImport.js config [accumulo]

["utafrali (via GitHub)" <gi...@apache.org>]

utafrali opened a new pull request, #6301:
URL: https://github.com/apache/accumulo/pull/6301

   Ran into some inconsistent spacing in the DataTables config. Aligned the indentation in the columns array to match the rest of the file.


-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix indentation in bulkImport.js config [accumulo]

["utafrali (via GitHub)" <gi...@apache.org>]

utafrali closed pull request #6301: Fix indentation in bulkImport.js config
URL: https://github.com/apache/accumulo/pull/6301


-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix indentation in bulkImport.js config [accumulo]

["utafrali (via GitHub)" <gi...@apache.org>]

utafrali commented on PR #6301:
URL: https://github.com/apache/accumulo/pull/6301#issuecomment-4209996907

   ah got it, was looking at the wrong part of the code. you're right that the live tservers set is where the actual bug is. closing this.


-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix indentation in bulkImport.js config [accumulo]

["keith-turner (via GitHub)" <gi...@apache.org>]

keith-turner commented on code in PR #6301:
URL: https://github.com/apache/accumulo/pull/6301#discussion_r3052362877


##########
server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java:
##########
@@ -229,9 +229,9 @@ private long removeTabletServerMarkers(Map<UUID,TServerInstance> uidMap,
     } catch (Exception ex) {
       throw new RuntimeException(ex);
     }
-    // remove parent znode for dead tablet servers
+    // remove parent znode for dead tablet servers that had WALs
     for (Entry<TServerInstance,Set<UUID>> entry : candidates.entrySet()) {
-      if (!liveServers.contains(entry.getKey())) {
+      if (!liveServers.contains(entry.getKey()) && !entry.getValue().isEmpty()) {

Review Comment:
   This change does not fix #6298 and it could prevent cleaning up data in zookeeper that needs to be removed.  The bug in #6298 was caused by the set of live tservers being incorrect.  When the set of live tservers is correct, then this code is correct as is.  
   
   For background the information stored in zookeeper is used to determine which logs are in use by a live tserver.  When we get to this point in the code if a tserver is dead and it has no references in the metadata table to any of it walogs (this is checked in earlier code), then we must remove everything in zookeeper because the information is no longer needed and leaving in ZK would eventually cause memory problems for ZK. 



-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org