You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "keith-turner (via GitHub)" <gi...@apache.org> on 2023/04/18 18:36:53 UTC
[GitHub] [accumulo] keith-turner opened a new pull request, #3317: fixes #3284 updates tablet locs using conditional mutations
keith-turner opened a new pull request, #3317:
URL: https://github.com/apache/accumulo/pull/3317
Updates tablet locations using conditional mutations. Two supporting changes were made and one bug was fixes while making this change.
The first supporting change was streamlining handling of conditional mutations with a result of UNKNOWN. An UNKNOWN result on a conditional mutation occurs when the RPC for the conditional mutation has an error. In this case the conditional mutation may or may not have gone through. The tablet must be read to know what happened. This update adds support for automatically reading the tablet and checking it via a lambda. This makes it easy to write code for handling the unknown case.
The second supporting change was combining code that was mostly the same in ZooTabletStateStore and MetaDataStateStore by making both extend AbstractStateStore and use common code. This change allowed the updates to use conditional mutations to be made in one place instead of two.
The bug was with the new conditional writer code, it only supported writing tablets of the same table. The code was changed to only require that tablets be on the same datalevel. This change allowed ITs that create multiple tables to run.
Some places in the code that set locations were not changed to use conditional mutations. Comments were placed in the code for these.
--
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
[GitHub] [accumulo] dlmarion commented on a diff in pull request #3317: fixes #3284 updates tablet locs using conditional mutations
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #3317:
URL: https://github.com/apache/accumulo/pull/3317#discussion_r1181672471
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/AbstractTabletStateStore.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.manager.state;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.accumulo.core.client.ConditionalWriter;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.dataImpl.KeyExtent;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.TabletLocationState;
+import org.apache.accumulo.core.metadata.schema.Ample;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.tabletserver.log.LogEntry;
+import org.apache.accumulo.server.util.ManagerMetadataUtil;
+import org.apache.hadoop.fs.Path;
+
+import com.google.common.base.Preconditions;
+
+public abstract class AbstractTabletStateStore implements TabletStateStore {
+
+ private final ClientContext context;
+ private final Ample ample;
+
+ protected AbstractTabletStateStore(ClientContext context) {
+ this.context = context;
+ this.ample = context.getAmple();
+ }
+
+ @Override
+ public void setLocations(Collection<Assignment> assignments) throws DistributedStoreException {
+ try (var tabletsMutator = ample.conditionallyMutateTablets()) {
+ for (Assignment assignment : assignments) {
+ var conditionalMutator = tabletsMutator.mutateTablet(assignment.tablet)
+ .requireAbsentOperation()
+ .requireLocation(TabletMetadata.Location.future(assignment.server))
+ .putLocation(TabletMetadata.Location.current(assignment.server))
+ .deleteLocation(TabletMetadata.Location.future(assignment.server)).deleteSuspension();
Review Comment:
ok, so those sort in the manner in which they are specified since the sort parameters are the same?
--
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
[GitHub] [accumulo] keith-turner commented on a diff in pull request #3317: fixes #3284 updates tablet locs using conditional mutations
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3317:
URL: https://github.com/apache/accumulo/pull/3317#discussion_r1181745780
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/AbstractTabletStateStore.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.manager.state;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.accumulo.core.client.ConditionalWriter;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.dataImpl.KeyExtent;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.TabletLocationState;
+import org.apache.accumulo.core.metadata.schema.Ample;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.tabletserver.log.LogEntry;
+import org.apache.accumulo.server.util.ManagerMetadataUtil;
+import org.apache.hadoop.fs.Path;
+
+import com.google.common.base.Preconditions;
+
+public abstract class AbstractTabletStateStore implements TabletStateStore {
+
+ private final ClientContext context;
+ private final Ample ample;
+
+ protected AbstractTabletStateStore(ClientContext context) {
+ this.context = context;
+ this.ample = context.getAmple();
+ }
+
+ @Override
+ public void setLocations(Collection<Assignment> assignments) throws DistributedStoreException {
+ try (var tabletsMutator = ample.conditionallyMutateTablets()) {
+ for (Assignment assignment : assignments) {
+ var conditionalMutator = tabletsMutator.mutateTablet(assignment.tablet)
+ .requireAbsentOperation()
+ .requireLocation(TabletMetadata.Location.future(assignment.server))
+ .putLocation(TabletMetadata.Location.current(assignment.server))
+ .deleteLocation(TabletMetadata.Location.future(assignment.server)).deleteSuspension();
Review Comment:
> Are all of the conditions checked before any changes are applied and therefore the order is irrelevant?
That's correct. The code that sorts the conditions just does that to make them more efficient to check, but it does not have to be done for correctness.
--
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
[GitHub] [accumulo] dlmarion commented on a diff in pull request #3317: fixes #3284 updates tablet locs using conditional mutations
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #3317:
URL: https://github.com/apache/accumulo/pull/3317#discussion_r1181694611
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/AbstractTabletStateStore.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.manager.state;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.accumulo.core.client.ConditionalWriter;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.dataImpl.KeyExtent;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.TabletLocationState;
+import org.apache.accumulo.core.metadata.schema.Ample;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.tabletserver.log.LogEntry;
+import org.apache.accumulo.server.util.ManagerMetadataUtil;
+import org.apache.hadoop.fs.Path;
+
+import com.google.common.base.Preconditions;
+
+public abstract class AbstractTabletStateStore implements TabletStateStore {
+
+ private final ClientContext context;
+ private final Ample ample;
+
+ protected AbstractTabletStateStore(ClientContext context) {
+ this.context = context;
+ this.ample = context.getAmple();
+ }
+
+ @Override
+ public void setLocations(Collection<Assignment> assignments) throws DistributedStoreException {
+ try (var tabletsMutator = ample.conditionallyMutateTablets()) {
+ for (Assignment assignment : assignments) {
+ var conditionalMutator = tabletsMutator.mutateTablet(assignment.tablet)
+ .requireAbsentOperation()
+ .requireLocation(TabletMetadata.Location.future(assignment.server))
+ .putLocation(TabletMetadata.Location.current(assignment.server))
+ .deleteLocation(TabletMetadata.Location.future(assignment.server)).deleteSuspension();
Review Comment:
Sorry for asking so many questions about this, I'm not familiar with ConditionalMutations. In the code above there is an implied order in the conditions, but it appears that they are re-sorted on the client side before sending to the server [here](https://github.com/apache/accumulo/blob/ba472d6e24daa8f0014a22cabace3061f5d46413/core/src/main/java/org/apache/accumulo/core/clientImpl/ConditionalWriterImpl.java#L777) which is called as part of the write process in `ConditionalWriterImpl.sendToServer`.
Are all of the conditions checked before any changes are applied and therefore the order is irrelevant?
--
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
[GitHub] [accumulo] keith-turner commented on a diff in pull request #3317: fixes #3284 updates tablet locs using conditional mutations
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3317:
URL: https://github.com/apache/accumulo/pull/3317#discussion_r1181745780
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/AbstractTabletStateStore.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.manager.state;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.accumulo.core.client.ConditionalWriter;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.dataImpl.KeyExtent;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.TabletLocationState;
+import org.apache.accumulo.core.metadata.schema.Ample;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.tabletserver.log.LogEntry;
+import org.apache.accumulo.server.util.ManagerMetadataUtil;
+import org.apache.hadoop.fs.Path;
+
+import com.google.common.base.Preconditions;
+
+public abstract class AbstractTabletStateStore implements TabletStateStore {
+
+ private final ClientContext context;
+ private final Ample ample;
+
+ protected AbstractTabletStateStore(ClientContext context) {
+ this.context = context;
+ this.ample = context.getAmple();
+ }
+
+ @Override
+ public void setLocations(Collection<Assignment> assignments) throws DistributedStoreException {
+ try (var tabletsMutator = ample.conditionallyMutateTablets()) {
+ for (Assignment assignment : assignments) {
+ var conditionalMutator = tabletsMutator.mutateTablet(assignment.tablet)
+ .requireAbsentOperation()
+ .requireLocation(TabletMetadata.Location.future(assignment.server))
+ .putLocation(TabletMetadata.Location.current(assignment.server))
+ .deleteLocation(TabletMetadata.Location.future(assignment.server)).deleteSuspension();
Review Comment:
> Are all of the conditions checked before any changes are applied and therefore the order is irrelevant?
That's correct. The code that sorts the conditions just does that to make them more efficient to check.
--
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
[GitHub] [accumulo] keith-turner commented on a diff in pull request #3317: fixes #3284 updates tablet locs using conditional mutations
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3317:
URL: https://github.com/apache/accumulo/pull/3317#discussion_r1181667390
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java:
##########
@@ -398,9 +399,7 @@ ConditionalTabletMutator requireOperation(TabletOperation operation,
/**
* Convenience interface for handling conditional mutations with a status of UNKNOWN.
*/
- interface UknownValidator {
- boolean shouldAccept(TabletMetadata tabletMetadata);
- }
+ interface UknownValidator extends Predicate<TabletMetadata> {}
Review Comment:
updated in c5fe3e2
--
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
[GitHub] [accumulo] dlmarion commented on a diff in pull request #3317: fixes #3284 updates tablet locs using conditional mutations
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #3317:
URL: https://github.com/apache/accumulo/pull/3317#discussion_r1181567916
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/AbstractTabletStateStore.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.manager.state;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.accumulo.core.client.ConditionalWriter;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.dataImpl.KeyExtent;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.TabletLocationState;
+import org.apache.accumulo.core.metadata.schema.Ample;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.tabletserver.log.LogEntry;
+import org.apache.accumulo.server.util.ManagerMetadataUtil;
+import org.apache.hadoop.fs.Path;
+
+import com.google.common.base.Preconditions;
+
+public abstract class AbstractTabletStateStore implements TabletStateStore {
+
+ private final ClientContext context;
+ private final Ample ample;
+
+ protected AbstractTabletStateStore(ClientContext context) {
+ this.context = context;
+ this.ample = context.getAmple();
+ }
+
+ @Override
+ public void setLocations(Collection<Assignment> assignments) throws DistributedStoreException {
+ try (var tabletsMutator = ample.conditionallyMutateTablets()) {
+ for (Assignment assignment : assignments) {
+ var conditionalMutator = tabletsMutator.mutateTablet(assignment.tablet)
+ .requireAbsentOperation()
+ .requireLocation(TabletMetadata.Location.future(assignment.server))
+ .putLocation(TabletMetadata.Location.current(assignment.server))
+ .deleteLocation(TabletMetadata.Location.future(assignment.server)).deleteSuspension();
Review Comment:
It appears that `ConditionalWriter.convertConditions` will sort the Conditions based on CF, CQ, Visibility, and then timestamp. Given that `requireLocation`, `putLocation`, and `deleteLocation` all operate on the same CF and CQ with a timestamp of 0L, do we know in which order they are applied?
--
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
[GitHub] [accumulo] keith-turner commented on a diff in pull request #3317: fixes #3284 updates tablet locs using conditional mutations
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3317:
URL: https://github.com/apache/accumulo/pull/3317#discussion_r1181680120
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/AbstractTabletStateStore.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.manager.state;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.accumulo.core.client.ConditionalWriter;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.dataImpl.KeyExtent;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.TabletLocationState;
+import org.apache.accumulo.core.metadata.schema.Ample;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.tabletserver.log.LogEntry;
+import org.apache.accumulo.server.util.ManagerMetadataUtil;
+import org.apache.hadoop.fs.Path;
+
+import com.google.common.base.Preconditions;
+
+public abstract class AbstractTabletStateStore implements TabletStateStore {
+
+ private final ClientContext context;
+ private final Ample ample;
+
+ protected AbstractTabletStateStore(ClientContext context) {
+ this.context = context;
+ this.ample = context.getAmple();
+ }
+
+ @Override
+ public void setLocations(Collection<Assignment> assignments) throws DistributedStoreException {
+ try (var tabletsMutator = ample.conditionallyMutateTablets()) {
+ for (Assignment assignment : assignments) {
+ var conditionalMutator = tabletsMutator.mutateTablet(assignment.tablet)
+ .requireAbsentOperation()
+ .requireLocation(TabletMetadata.Location.future(assignment.server))
+ .putLocation(TabletMetadata.Location.current(assignment.server))
+ .deleteLocation(TabletMetadata.Location.future(assignment.server)).deleteSuspension();
Review Comment:
The current and future locations have different col families.
https://github.com/apache/accumulo/blob/ba472d6e24daa8f0014a22cabace3061f5d46413/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java#L132-L151
--
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
[GitHub] [accumulo] keith-turner commented on a diff in pull request #3317: fixes #3284 updates tablet locs using conditional mutations
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3317:
URL: https://github.com/apache/accumulo/pull/3317#discussion_r1181683689
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/AbstractTabletStateStore.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.manager.state;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.accumulo.core.client.ConditionalWriter;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.dataImpl.KeyExtent;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.TabletLocationState;
+import org.apache.accumulo.core.metadata.schema.Ample;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.tabletserver.log.LogEntry;
+import org.apache.accumulo.server.util.ManagerMetadataUtil;
+import org.apache.hadoop.fs.Path;
+
+import com.google.common.base.Preconditions;
+
+public abstract class AbstractTabletStateStore implements TabletStateStore {
+
+ private final ClientContext context;
+ private final Ample ample;
+
+ protected AbstractTabletStateStore(ClientContext context) {
+ this.context = context;
+ this.ample = context.getAmple();
+ }
+
+ @Override
+ public void setLocations(Collection<Assignment> assignments) throws DistributedStoreException {
+ try (var tabletsMutator = ample.conditionallyMutateTablets()) {
+ for (Assignment assignment : assignments) {
+ var conditionalMutator = tabletsMutator.mutateTablet(assignment.tablet)
+ .requireAbsentOperation()
+ .requireLocation(TabletMetadata.Location.future(assignment.server))
+ .putLocation(TabletMetadata.Location.current(assignment.server))
+ .deleteLocation(TabletMetadata.Location.future(assignment.server)).deleteSuspension();
Review Comment:
So what happens on the server is
1. it gets a lock on the row
2. it checks the cf=future and cq=session match what was specified in requireLocation(TabletMetadata.Location.future(assignment.server))
3. If the above check was succesful it deletes the future col and sets the current col
4. releases the row lock
--
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
[GitHub] [accumulo] keith-turner merged pull request #3317: fixes #3284 updates tablet locs using conditional mutations
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner merged PR #3317:
URL: https://github.com/apache/accumulo/pull/3317
--
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
[GitHub] [accumulo] keith-turner commented on a diff in pull request #3317: fixes #3284 updates tablet locs using conditional mutations
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3317:
URL: https://github.com/apache/accumulo/pull/3317#discussion_r1181680120
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/AbstractTabletStateStore.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.manager.state;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.accumulo.core.client.ConditionalWriter;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.dataImpl.KeyExtent;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.TabletLocationState;
+import org.apache.accumulo.core.metadata.schema.Ample;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.tabletserver.log.LogEntry;
+import org.apache.accumulo.server.util.ManagerMetadataUtil;
+import org.apache.hadoop.fs.Path;
+
+import com.google.common.base.Preconditions;
+
+public abstract class AbstractTabletStateStore implements TabletStateStore {
+
+ private final ClientContext context;
+ private final Ample ample;
+
+ protected AbstractTabletStateStore(ClientContext context) {
+ this.context = context;
+ this.ample = context.getAmple();
+ }
+
+ @Override
+ public void setLocations(Collection<Assignment> assignments) throws DistributedStoreException {
+ try (var tabletsMutator = ample.conditionallyMutateTablets()) {
+ for (Assignment assignment : assignments) {
+ var conditionalMutator = tabletsMutator.mutateTablet(assignment.tablet)
+ .requireAbsentOperation()
+ .requireLocation(TabletMetadata.Location.future(assignment.server))
+ .putLocation(TabletMetadata.Location.current(assignment.server))
+ .deleteLocation(TabletMetadata.Location.future(assignment.server)).deleteSuspension();
Review Comment:
The current and future locations have different col families.
--
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
[GitHub] [accumulo] dlmarion commented on a diff in pull request #3317: fixes #3284 updates tablet locs using conditional mutations
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #3317:
URL: https://github.com/apache/accumulo/pull/3317#discussion_r1181749735
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/AbstractTabletStateStore.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.manager.state;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.accumulo.core.client.ConditionalWriter;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.dataImpl.KeyExtent;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.TabletLocationState;
+import org.apache.accumulo.core.metadata.schema.Ample;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.tabletserver.log.LogEntry;
+import org.apache.accumulo.server.util.ManagerMetadataUtil;
+import org.apache.hadoop.fs.Path;
+
+import com.google.common.base.Preconditions;
+
+public abstract class AbstractTabletStateStore implements TabletStateStore {
+
+ private final ClientContext context;
+ private final Ample ample;
+
+ protected AbstractTabletStateStore(ClientContext context) {
+ this.context = context;
+ this.ample = context.getAmple();
+ }
+
+ @Override
+ public void setLocations(Collection<Assignment> assignments) throws DistributedStoreException {
+ try (var tabletsMutator = ample.conditionallyMutateTablets()) {
+ for (Assignment assignment : assignments) {
+ var conditionalMutator = tabletsMutator.mutateTablet(assignment.tablet)
+ .requireAbsentOperation()
+ .requireLocation(TabletMetadata.Location.future(assignment.server))
+ .putLocation(TabletMetadata.Location.current(assignment.server))
+ .deleteLocation(TabletMetadata.Location.future(assignment.server)).deleteSuspension();
Review Comment:
ok, thanks. I think that alleviates my concerns.
--
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
[GitHub] [accumulo] keith-turner commented on a diff in pull request #3317: fixes #3284 updates tablet locs using conditional mutations
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3317:
URL: https://github.com/apache/accumulo/pull/3317#discussion_r1181659106
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/AbstractTabletStateStore.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.manager.state;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.accumulo.core.client.ConditionalWriter;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.dataImpl.KeyExtent;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.TabletLocationState;
+import org.apache.accumulo.core.metadata.schema.Ample;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.tabletserver.log.LogEntry;
+import org.apache.accumulo.server.util.ManagerMetadataUtil;
+import org.apache.hadoop.fs.Path;
+
+import com.google.common.base.Preconditions;
+
+public abstract class AbstractTabletStateStore implements TabletStateStore {
+
+ private final ClientContext context;
+ private final Ample ample;
+
+ protected AbstractTabletStateStore(ClientContext context) {
+ this.context = context;
+ this.ample = context.getAmple();
+ }
+
+ @Override
+ public void setLocations(Collection<Assignment> assignments) throws DistributedStoreException {
+ try (var tabletsMutator = ample.conditionallyMutateTablets()) {
+ for (Assignment assignment : assignments) {
+ var conditionalMutator = tabletsMutator.mutateTablet(assignment.tablet)
+ .requireAbsentOperation()
+ .requireLocation(TabletMetadata.Location.future(assignment.server))
+ .putLocation(TabletMetadata.Location.current(assignment.server))
+ .deleteLocation(TabletMetadata.Location.future(assignment.server)).deleteSuspension();
Review Comment:
The requireLocation call will create a condition on the conditional mutation. All conditions on a conditional mutation are checked first before any changes are made. So a check is done to see if the future location is as expected and if it is then the changes to delete the future and set the current are made. If the check of the current future location fails then no changes are made.
--
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
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #3317: fixes #3284 updates tablet locs using conditional mutations
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #3317:
URL: https://github.com/apache/accumulo/pull/3317#discussion_r1179727819
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java:
##########
@@ -398,9 +399,7 @@ ConditionalTabletMutator requireOperation(TabletOperation operation,
/**
* Convenience interface for handling conditional mutations with a status of UNKNOWN.
*/
- interface UknownValidator {
- boolean shouldAccept(TabletMetadata tabletMetadata);
- }
+ interface UknownValidator extends Predicate<TabletMetadata> {}
Review Comment:
Should this typo be fixed?
```suggestion
interface UnknownValidator extends Predicate<TabletMetadata> {}
```
--
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
[GitHub] [accumulo] keith-turner commented on a diff in pull request #3317: fixes #3284 updates tablet locs using conditional mutations
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3317:
URL: https://github.com/apache/accumulo/pull/3317#discussion_r1181745780
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/AbstractTabletStateStore.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.manager.state;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.accumulo.core.client.ConditionalWriter;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.dataImpl.KeyExtent;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.TabletLocationState;
+import org.apache.accumulo.core.metadata.schema.Ample;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.tabletserver.log.LogEntry;
+import org.apache.accumulo.server.util.ManagerMetadataUtil;
+import org.apache.hadoop.fs.Path;
+
+import com.google.common.base.Preconditions;
+
+public abstract class AbstractTabletStateStore implements TabletStateStore {
+
+ private final ClientContext context;
+ private final Ample ample;
+
+ protected AbstractTabletStateStore(ClientContext context) {
+ this.context = context;
+ this.ample = context.getAmple();
+ }
+
+ @Override
+ public void setLocations(Collection<Assignment> assignments) throws DistributedStoreException {
+ try (var tabletsMutator = ample.conditionallyMutateTablets()) {
+ for (Assignment assignment : assignments) {
+ var conditionalMutator = tabletsMutator.mutateTablet(assignment.tablet)
+ .requireAbsentOperation()
+ .requireLocation(TabletMetadata.Location.future(assignment.server))
+ .putLocation(TabletMetadata.Location.current(assignment.server))
+ .deleteLocation(TabletMetadata.Location.future(assignment.server)).deleteSuspension();
Review Comment:
> Are all of the conditions checked before any changes are applied and therefore the order is irrelevant?
That's correct. The code that sorts the conditions just does that to make them more efficient to check, but it does not have to 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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org