keith-turner commented on code in PR #4160:
URL: https://github.com/apache/accumulo/pull/4160#discussion_r1459724592
##########
core/src/main/java/org/apache/accumulo/core/fate/accumulo/FateMutatorImpl.java:
##########
@@ -132,12 +140,30 @@ public FateMutator<T> delete() {
return this;
}
+ /**
+ * Require that the transaction status is one of the given statuses. If no
statuses are provided,
+ * require that the status column is absent.
+ *
+ * @param statuses The statuses to check against.
+ */
+ public FateMutator<T> requireStatus(TStatus... statuses) {
+ Condition condition = StatusMappingIterator.createCondition(statuses);
+ mutation.addCondition(condition);
+ return this;
+ }
+
@Override
public void mutate() {
- try (BatchWriter writer = context.createBatchWriter(tableName)) {
- writer.addMutation(mutation);
- } catch (Exception e) {
- throw new IllegalStateException(e);
+ try (ConditionalWriter writer =
context.createConditionalWriter(tableName)) {
Review Comment:
For this method if there are not conditions, we could use a batch writer for
the mutation instead of adding the condition for them empty column.
```suggestion
if(mutation.getConditions().isEmpty()) {
// use batch writer
} else {
// use conditional writer
}
```
##########
core/src/main/java/org/apache/accumulo/core/fate/accumulo/AccumuloStore.java:
##########
@@ -70,7 +70,8 @@ public AccumuloStore(ClientContext context) {
public long create() {
long tid = RANDOM.get().nextLong() & 0x7fffffffffffffffL;
-
newMutator(tid).putStatus(TStatus.NEW).putCreateTime(System.currentTimeMillis()).mutate();
+
newMutator(tid).requireStatus().putStatus(TStatus.NEW).putCreateTime(System.currentTimeMillis())
Review Comment:
We need to retry when this fails because the conditiona was not accepted.
Not sure of the best way to do this. Could have a special exception and catch
that OR make mutate() return something. Also need to test this if possible.
Maybe we could make the next id a function that can be overridden by a test
class.
```java
// exist to be overridden by test so can test trying to create existing id
protected long getNextId() {
return RANDOM.get().nextLong() & 0x7fffffffffffffffL;
}
public long create() {
long tid = getNextId();
while(true) {
// attempt to persist id
// if successful return tid
/// if not successful then try again : tid = getNextId();
//sleep or use retry obj
}
```
Feel free to ignore the comments about testing. Not sure about my testing
comment, maybe testing needs to be at a lower level like is already being done.
Even w/o the changes for testing, do need to retry somehow.
--
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]
