You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "dlmarion (via GitHub)" <gi...@apache.org> on 2024/03/20 12:47:12 UTC

[PR] Modified removeUnusedWALEntries to use conditional mutations [accumulo]

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

   (no comment)


-- 
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] Modified removeUnusedWALEntries to use conditional mutations [accumulo]

Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #4404:
URL: https://github.com/apache/accumulo/pull/4404#discussion_r1535732447


##########
server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java:
##########
@@ -246,10 +250,19 @@ public static void deleteTable(TableId tableId, boolean insertDeletes, ServerCon
 
   public static void removeUnusedWALEntries(ServerContext context, KeyExtent extent,
       final Collection<LogEntry> entries, ServiceLock zooLock) {
-    TabletMutator tablet = context.getAmple().mutateTablet(extent);
-    entries.forEach(tablet::deleteWal);
-    tablet.putZooLock(context.getZooKeeperRoot(), zooLock);
-    tablet.mutate();
+    try (ConditionalTabletsMutator mutator = context.getAmple().conditionallyMutateTablets()) {
+      ConditionalTabletMutator mut = mutator.mutateTablet(extent).requireAbsentOperation()

Review Comment:
   Given my comment above, I don't see anything in the code that removes the lock column from the metadata. So, adding something like `requireAbsentLock` may not work.



-- 
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] Modified removeUnusedWALEntries to use conditional mutations [accumulo]

Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on code in PR #4404:
URL: https://github.com/apache/accumulo/pull/4404#discussion_r1535712788


##########
server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java:
##########
@@ -246,10 +250,19 @@ public static void deleteTable(TableId tableId, boolean insertDeletes, ServerCon
 
   public static void removeUnusedWALEntries(ServerContext context, KeyExtent extent,
       final Collection<LogEntry> entries, ServiceLock zooLock) {
-    TabletMutator tablet = context.getAmple().mutateTablet(extent);
-    entries.forEach(tablet::deleteWal);
-    tablet.putZooLock(context.getZooKeeperRoot(), zooLock);
-    tablet.mutate();
+    try (ConditionalTabletsMutator mutator = context.getAmple().conditionallyMutateTablets()) {
+      ConditionalTabletMutator mut = mutator.mutateTablet(extent).requireAbsentOperation()

Review Comment:
   I think you need to add a condition here to require for the conditional mutation. The condition you have is only set for the rejection handler. The rejection handler I believe is only used to check if it should succeed if it was reported as rejected but not check if it was successful. So I think you need something like the following which would require absent logs and then you'd need to implement requireLock()
   
   ```java
         ConditionalTabletMutator mut = mutator.mutateTablet(extent).requireAbsentOperation()
             .requireAbsentLogs().requireLock(zooLock)
             .putZooLock(context.getZooKeeperRoot(), zooLock);
   ```
   
   



-- 
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] Modified removeUnusedWALEntries to use conditional mutations [accumulo]

Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on PR #4404:
URL: https://github.com/apache/accumulo/pull/4404#issuecomment-2030587231

   > This is likely to prevent a half-dead server from updating the metadata table.
   
   Yeah the lock column is a best effort attempt to avoid writes from dead servers.  Its not guaranteed to stop all writes because it use zoocache in the metadata constraint check, so the check in the metadata constraint is eventually consistent.


-- 
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] Modified removeUnusedWALEntries to use conditional mutations [accumulo]

Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on code in PR #4404:
URL: https://github.com/apache/accumulo/pull/4404#discussion_r1535712788


##########
server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java:
##########
@@ -246,10 +250,19 @@ public static void deleteTable(TableId tableId, boolean insertDeletes, ServerCon
 
   public static void removeUnusedWALEntries(ServerContext context, KeyExtent extent,
       final Collection<LogEntry> entries, ServiceLock zooLock) {
-    TabletMutator tablet = context.getAmple().mutateTablet(extent);
-    entries.forEach(tablet::deleteWal);
-    tablet.putZooLock(context.getZooKeeperRoot(), zooLock);
-    tablet.mutate();
+    try (ConditionalTabletsMutator mutator = context.getAmple().conditionallyMutateTablets()) {
+      ConditionalTabletMutator mut = mutator.mutateTablet(extent).requireAbsentOperation()

Review Comment:
   I think you need to add a condition here to require for the conditional mutation. The condition you have is only set for the rejection handler. The rejection handler I believe is only used to check if it should succeed if it was reported as rejected but not check if it was successful. So I think you need something like the following which would require absent logs:
   
   ```java
         ConditionalTabletMutator mut = mutator.mutateTablet(extent).requireAbsentOperation()
           // make sure absent logs
             .requireAbsentLogs()
             .putZooLock(context.getZooKeeperRoot(), zooLock);
   ```
   
   



##########
server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java:
##########
@@ -246,10 +250,19 @@ public static void deleteTable(TableId tableId, boolean insertDeletes, ServerCon
 
   public static void removeUnusedWALEntries(ServerContext context, KeyExtent extent,
       final Collection<LogEntry> entries, ServiceLock zooLock) {
-    TabletMutator tablet = context.getAmple().mutateTablet(extent);
-    entries.forEach(tablet::deleteWal);
-    tablet.putZooLock(context.getZooKeeperRoot(), zooLock);
-    tablet.mutate();
+    try (ConditionalTabletsMutator mutator = context.getAmple().conditionallyMutateTablets()) {
+      ConditionalTabletMutator mut = mutator.mutateTablet(extent).requireAbsentOperation()

Review Comment:
   Edit: I meant to say in my previous post before I modified it it should require NOT having the current zooLock since obviously it's setting the current one, but that may not matter. I just noticed in the rejection handler you were checking that the zooLock equals the current lock, so I was thinking we should require there not be a lock already when submitting but this might not be necessary 



-- 
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] Modified removeUnusedWALEntries to use conditional mutations [accumulo]

Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on code in PR #4404:
URL: https://github.com/apache/accumulo/pull/4404#discussion_r1535726287


##########
server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java:
##########
@@ -246,10 +250,19 @@ public static void deleteTable(TableId tableId, boolean insertDeletes, ServerCon
 
   public static void removeUnusedWALEntries(ServerContext context, KeyExtent extent,
       final Collection<LogEntry> entries, ServiceLock zooLock) {
-    TabletMutator tablet = context.getAmple().mutateTablet(extent);
-    entries.forEach(tablet::deleteWal);
-    tablet.putZooLock(context.getZooKeeperRoot(), zooLock);
-    tablet.mutate();
+    try (ConditionalTabletsMutator mutator = context.getAmple().conditionallyMutateTablets()) {
+      ConditionalTabletMutator mut = mutator.mutateTablet(extent).requireAbsentOperation()

Review Comment:
   Edit: I meant to say in my previous post before I modified it it should require NOT having the current zooLock since obviously it's setting the current one, but that may not matter. I just noticed in the rejection handler you were checking that the zooLock set equals the current lock, so I was thinking we should require there not be a lock already when submitting but this might not be necessary 



-- 
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] Modified removeUnusedWALEntries to use conditional mutations [accumulo]

Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #4404:
URL: https://github.com/apache/accumulo/pull/4404#discussion_r1546937999


##########
server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java:
##########
@@ -246,10 +250,19 @@ public static void deleteTable(TableId tableId, boolean insertDeletes, ServerCon
 
   public static void removeUnusedWALEntries(ServerContext context, KeyExtent extent,
       final Collection<LogEntry> entries, ServiceLock zooLock) {
-    TabletMutator tablet = context.getAmple().mutateTablet(extent);
-    entries.forEach(tablet::deleteWal);
-    tablet.putZooLock(context.getZooKeeperRoot(), zooLock);
-    tablet.mutate();
+    try (ConditionalTabletsMutator mutator = context.getAmple().conditionallyMutateTablets()) {
+      ConditionalTabletMutator mut = mutator.mutateTablet(extent).requireAbsentOperation()

Review Comment:
   Looking through the code,  this is called by the tablet during recovery before the location on the tablet is set.  So at the point when this is called seems like the tablet is expected to have a future location of the current tablet server.  We can check for that in the conditional update.
   
   So maybe could do something like the following.  However, the tablet server object is not available here.  I looked at some other code in Tablet class to see how to create the expected location and that code called `tabletServer.getTabletSession()`. Wondering if this function should be moved to the Tablet class since only the Tablet class calls it.
   
   ```suggestion
         Location expectedLocation = Location.future(tabletServer.getTabletSession());
         ConditionalTabletMutator mut = mutator.mutateTablet(extent).requireAbsentOperation().requireLocation(expectedLocation)
   ```



##########
server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java:
##########
@@ -246,10 +250,19 @@ public static void deleteTable(TableId tableId, boolean insertDeletes, ServerCon
 
   public static void removeUnusedWALEntries(ServerContext context, KeyExtent extent,
       final Collection<LogEntry> entries, ServiceLock zooLock) {
-    TabletMutator tablet = context.getAmple().mutateTablet(extent);
-    entries.forEach(tablet::deleteWal);
-    tablet.putZooLock(context.getZooKeeperRoot(), zooLock);
-    tablet.mutate();
+    try (ConditionalTabletsMutator mutator = context.getAmple().conditionallyMutateTablets()) {
+      ConditionalTabletMutator mut = mutator.mutateTablet(extent).requireAbsentOperation()
+          .putZooLock(context.getZooKeeperRoot(), zooLock);
+      entries.forEach(mut::deleteWal);
+      mut.submit(tabletMetadata -> tabletMetadata.getLock()
+          .equals(zooLock.getLockID().serialize(context.getZooKeeperRoot() + "/"))
+          && (tabletMetadata.getLogs() == null || tabletMetadata.getLogs().isEmpty()));

Review Comment:
   Checking the lock in submit could overlap in time with other tablets updates that set the lock also.  So should probably not check that.  For example if a concurrent compaction update for a tablet happened to the set the lock to the manager process lock info.
   
   Also the `getLogs()` function on TabletMetadata should never return null and if does its probably a bug and it would be ok to throw an NPE in that case.  So should probably not check for null and consider it a success.
   
   ```suggestion
         mut.submit(tabletMetadata -> tabletMetadata.getLogs().isEmpty());
   ```



-- 
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] Modified removeUnusedWALEntries to use conditional mutations [accumulo]

Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on code in PR #4404:
URL: https://github.com/apache/accumulo/pull/4404#discussion_r1535712788


##########
server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java:
##########
@@ -246,10 +250,19 @@ public static void deleteTable(TableId tableId, boolean insertDeletes, ServerCon
 
   public static void removeUnusedWALEntries(ServerContext context, KeyExtent extent,
       final Collection<LogEntry> entries, ServiceLock zooLock) {
-    TabletMutator tablet = context.getAmple().mutateTablet(extent);
-    entries.forEach(tablet::deleteWal);
-    tablet.putZooLock(context.getZooKeeperRoot(), zooLock);
-    tablet.mutate();
+    try (ConditionalTabletsMutator mutator = context.getAmple().conditionallyMutateTablets()) {
+      ConditionalTabletMutator mut = mutator.mutateTablet(extent).requireAbsentOperation()

Review Comment:
   I think you need to add a condition here to require for the conditional mutation. The condition you have is only set for the rejection handler. The rejection handler I believe is only used to check if it should succeed if it was reported as rejected but not check if it was successful. So I think you need something like the following which would require absent logs:
   
   ```java
         ConditionalTabletMutator mut = mutator.mutateTablet(extent).requireAbsentOperation()
           // make sure absent logs and 
             .requireAbsentLogs()
             .putZooLock(context.getZooKeeperRoot(), zooLock);
   ```
   
   



-- 
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] Modified removeUnusedWALEntries to use conditional mutations [accumulo]

Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on PR #4404:
URL: https://github.com/apache/accumulo/pull/4404#issuecomment-2020667541

   I did some looking at how `srv:lock` is used in #4420, and I don't think we can remove it. It looks like it's added to the Mutation by the client so that it can be evaluated server-side in MetadataConstraints. The server-side check confirms that the server that sent the mutation still holds a lock in ZooKeeper. This is likely to prevent a half-dead server from updating the metadata table.


-- 
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] Modified removeUnusedWALEntries to use conditional mutations [accumulo]

Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on code in PR #4404:
URL: https://github.com/apache/accumulo/pull/4404#discussion_r1535726287


##########
server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java:
##########
@@ -246,10 +250,19 @@ public static void deleteTable(TableId tableId, boolean insertDeletes, ServerCon
 
   public static void removeUnusedWALEntries(ServerContext context, KeyExtent extent,
       final Collection<LogEntry> entries, ServiceLock zooLock) {
-    TabletMutator tablet = context.getAmple().mutateTablet(extent);
-    entries.forEach(tablet::deleteWal);
-    tablet.putZooLock(context.getZooKeeperRoot(), zooLock);
-    tablet.mutate();
+    try (ConditionalTabletsMutator mutator = context.getAmple().conditionallyMutateTablets()) {
+      ConditionalTabletMutator mut = mutator.mutateTablet(extent).requireAbsentOperation()

Review Comment:
   Edit: I meant to say in my previous post before I modified it that to add that it should require NOT having the current zooLock since obviously it's setting the current one, but that may not matter. I just noticed in the rejection handler you were checking that the zooLock set equals the current lock, so I was thinking we should require there not be a lock already when submitting but this might not be necessary 



-- 
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] Modified removeUnusedWALEntries to use conditional mutations [accumulo]

Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on PR #4404:
URL: https://github.com/apache/accumulo/pull/4404#issuecomment-2015526298

   I took a look at the lock column and as far as I can tell it's never removed, at least not using the static schema variable. I searched around and don't see it cleared even on offline but not sure if I missed something.
   
   It's only used by MetadataConstraints on updates to [check](https://github.com/apache/accumulo/blob/98b994ce0ad7d67a0e9d028b227909f7b7882ac7/server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java#L382-L406) that the value being written matches the zookeeper instance. This also matches the [documentation](https://accumulo.apache.org/docs/2.x/troubleshooting/system-metadata-tables): 
   
   > srv:lock [] tservers/127.0.0.1:9997/zlock-0000000001\$13fe86cd27101e5 - This is the lock information for the tablet holding the present lock. This information is checked against zookeeper whenever this is updated, which prevents a metadata update from a tablet server that no longer holds its lock.
   
   So my impression is you probably don't need to add it to TabletMetadata at all or check it in the conditional mutation which would make the tests pass. If you do add it then the point of those test failures is to make sure that we "consider" the column for splits and merges. Since splits and merges can happen on tables that are not hosted in elasticity we don't really care about the lock column but I wonder if it should be cleaned up at least if the tablets are not hosted.
   
   The other comment I have is that the TODO makes mention of using conditional mutations for removeUnusedWALEntries but it doesn't specify what should be required to succeed. Besides requireAbsentOperation() I am wondering what other conditions to validate, if any.  I was thinking about this and I feel like we might want to make sure WALs are the same, so maybe the code should look like:
   
   ```java
         ConditionalTabletMutator mut = mutator.mutateTablet(extent).requireAbsentOperation()
             .requireSame(tm, LOGS)
             .putZooLock(context.getZooKeeperRoot(), zooLock);
         entries.forEach(mut::deleteWal);
         mut.submit(tabletMetadata -> tabletMetadata.getLogs() == null || tabletMetadata.getLogs().isEmpty());
   
   ```
   
   This would require passing in the TabletMetadata as an argument of course
   
   


-- 
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] Modified removeUnusedWALEntries to use conditional mutations [accumulo]

Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on PR #4404:
URL: https://github.com/apache/accumulo/pull/4404#issuecomment-2009500281

   @keith-turner -  A couple of issues here:
   
   1. I don't see any code that removes the lock column from the metadata. I'm assuming that this should happen when the Tablet is unloaded from the TabletServer? 
   
   2. MergeTabletsTest and UpdateTabletsTest fail with an error regarding the new lock column. Assuming that issue 1 is resolved and the lock column is removed from the metadata, then I assume that the lock column can be ignored in the merge and split code as it should not exist because the Tablet should be unloaded for the merge/split Fate operation.


-- 
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] Modified removeUnusedWALEntries to use conditional mutations [accumulo]

Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion merged PR #4404:
URL: https://github.com/apache/accumulo/pull/4404


-- 
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] Modified removeUnusedWALEntries to use conditional mutations [accumulo]

Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on PR #4404:
URL: https://github.com/apache/accumulo/pull/4404#issuecomment-2015837594

   I'm wondering if we can just remove the lock column now from the tablet metadata. If its purpose was to restrict Tablet metadata updates to the TabletServer hosting the Tablet, that restriction no longer applies in elasticity.


-- 
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] Modified removeUnusedWALEntries to use conditional mutations [accumulo]

Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on code in PR #4404:
URL: https://github.com/apache/accumulo/pull/4404#discussion_r1535712788


##########
server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java:
##########
@@ -246,10 +250,19 @@ public static void deleteTable(TableId tableId, boolean insertDeletes, ServerCon
 
   public static void removeUnusedWALEntries(ServerContext context, KeyExtent extent,
       final Collection<LogEntry> entries, ServiceLock zooLock) {
-    TabletMutator tablet = context.getAmple().mutateTablet(extent);
-    entries.forEach(tablet::deleteWal);
-    tablet.putZooLock(context.getZooKeeperRoot(), zooLock);
-    tablet.mutate();
+    try (ConditionalTabletsMutator mutator = context.getAmple().conditionallyMutateTablets()) {
+      ConditionalTabletMutator mut = mutator.mutateTablet(extent).requireAbsentOperation()

Review Comment:
   I think you need to add a condition here to require for the conditional mutation. The condition you have is only set for the rejection handler. The rejection handler I believe is only used to check if it should succeed if it was reported as rejected but not check if it was successful. So I think you need something like the following which would require absent logs:
   
   ```java
         ConditionalTabletMutator mut = mutator.mutateTablet(extent).requireAbsentOperation()
           // make sure absent logs
             .requireAbsentLogs()
             .putZooLock(context.getZooKeeperRoot(), zooLock);
   ```
   
   



-- 
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] Modified removeUnusedWALEntries to use conditional mutations [accumulo]

Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on code in PR #4404:
URL: https://github.com/apache/accumulo/pull/4404#discussion_r1535726287


##########
server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java:
##########
@@ -246,10 +250,19 @@ public static void deleteTable(TableId tableId, boolean insertDeletes, ServerCon
 
   public static void removeUnusedWALEntries(ServerContext context, KeyExtent extent,
       final Collection<LogEntry> entries, ServiceLock zooLock) {
-    TabletMutator tablet = context.getAmple().mutateTablet(extent);
-    entries.forEach(tablet::deleteWal);
-    tablet.putZooLock(context.getZooKeeperRoot(), zooLock);
-    tablet.mutate();
+    try (ConditionalTabletsMutator mutator = context.getAmple().conditionallyMutateTablets()) {
+      ConditionalTabletMutator mut = mutator.mutateTablet(extent).requireAbsentOperation()

Review Comment:
   Edit: I meant to say in my previous post before I modified it it should require NOT having the current zooLock since obviously it's setting the current one, but that may not matter. I just noticed in the rejection handler you were checking that the zooLock equals the current lock, so I was thinking we should require there not be a lock already when submitting but this might not be necessary 



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