-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support implicit pre-read in Consensus Commit #1222
Changes from 1 commit
077335e
b658c70
ed8cc7a
e5768fd
a7c0071
84e26ae
a5bdbe7
460a579
2cfe9ae
be6babe
7714208
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,9 @@ | |
import com.scalar.db.api.Selection; | ||
import com.scalar.db.common.AbstractDistributedTransaction; | ||
import com.scalar.db.exception.storage.ExecutionException; | ||
import com.scalar.db.exception.transaction.CommitConflictException; | ||
import com.scalar.db.exception.transaction.CommitException; | ||
import com.scalar.db.exception.transaction.CrudConflictException; | ||
import com.scalar.db.exception.transaction.CrudException; | ||
import com.scalar.db.exception.transaction.UnknownTransactionStatusException; | ||
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; | ||
|
@@ -94,7 +96,7 @@ public void put(Put put) throws CrudException { | |
|
||
@Override | ||
public void put(List<Put> puts) throws CrudException { | ||
checkArgument(puts.size() != 0); | ||
checkArgument(!puts.isEmpty()); | ||
for (Put p : puts) { | ||
put(p); | ||
} | ||
|
@@ -109,15 +111,15 @@ public void delete(Delete delete) throws CrudException { | |
|
||
@Override | ||
public void delete(List<Delete> deletes) throws CrudException { | ||
checkArgument(deletes.size() != 0); | ||
checkArgument(!deletes.isEmpty()); | ||
for (Delete d : deletes) { | ||
delete(d); | ||
} | ||
} | ||
|
||
@Override | ||
public void mutate(List<? extends Mutation> mutations) throws CrudException { | ||
checkArgument(mutations.size() != 0); | ||
checkArgument(!mutations.isEmpty()); | ||
for (Mutation m : mutations) { | ||
if (m instanceof Put) { | ||
put((Put) m); | ||
|
@@ -129,6 +131,19 @@ public void mutate(List<? extends Mutation> mutations) throws CrudException { | |
|
||
@Override | ||
public void commit() throws CommitException, UnknownTransactionStatusException { | ||
// Fill the read set with records from the write and delete sets if they are unread | ||
try { | ||
crud.fillReadSetForRecordsFromWriteAndDeleteSetsIfUnread(); | ||
} catch (CrudConflictException e) { | ||
throw new CommitConflictException( | ||
"Conflict occurred while reading unread records in the write and delete sets", | ||
e, | ||
getId()); | ||
} catch (CrudException e) { | ||
throw new CommitException( | ||
"Failed to read unread records in the write and delete sets", e, getId()); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before committing (preparing) a transaction, we will fill the read set with records from the write and delete sets if they are unread. |
||
|
||
commit.commit(crud.getSnapshot()); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -30,6 +30,9 @@ public class ConsensusCommitConfig { | |||||
public static final String ASYNC_COMMIT_ENABLED = PREFIX + "async_commit.enabled"; | ||||||
public static final String ASYNC_ROLLBACK_ENABLED = PREFIX + "async_rollback.enabled"; | ||||||
|
||||||
public static final String PARALLEL_FILL_READ_SET_ENABLED = | ||||||
PREFIX + "parallel_fill_read_set.enabled"; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a difficult one...
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in ed8cc7a. It's renamed to |
||||||
|
||||||
public static final int DEFAULT_PARALLEL_EXECUTOR_COUNT = 128; | ||||||
|
||||||
public static final String INCLUDE_METADATA_ENABLED = PREFIX + "include_metadata.enabled"; | ||||||
|
@@ -48,6 +51,8 @@ public class ConsensusCommitConfig { | |||||
|
||||||
private final boolean isIncludeMetadataEnabled; | ||||||
|
||||||
private final boolean parallelFillReadSetEnabled; | ||||||
|
||||||
public ConsensusCommitConfig(DatabaseConfig databaseConfig) { | ||||||
String transactionManager = databaseConfig.getTransactionManager(); | ||||||
if (!"consensus-commit".equals(transactionManager)) { | ||||||
|
@@ -102,10 +107,16 @@ public ConsensusCommitConfig(DatabaseConfig databaseConfig) { | |||||
databaseConfig.getProperties(), PARALLEL_ROLLBACK_ENABLED, parallelCommitEnabled); | ||||||
|
||||||
asyncCommitEnabled = getBoolean(databaseConfig.getProperties(), ASYNC_COMMIT_ENABLED, false); | ||||||
|
||||||
// Use the value of async commit for async rollback as default value | ||||||
asyncRollbackEnabled = | ||||||
getBoolean(databaseConfig.getProperties(), ASYNC_ROLLBACK_ENABLED, asyncCommitEnabled); | ||||||
|
||||||
isIncludeMetadataEnabled = | ||||||
getBoolean(databaseConfig.getProperties(), INCLUDE_METADATA_ENABLED, false); | ||||||
|
||||||
parallelFillReadSetEnabled = | ||||||
getBoolean(databaseConfig.getProperties(), PARALLEL_FILL_READ_SET_ENABLED, true); | ||||||
} | ||||||
|
||||||
public Isolation getIsolation() { | ||||||
|
@@ -151,4 +162,8 @@ public boolean isAsyncRollbackEnabled() { | |||||
public boolean isIncludeMetadataEnabled() { | ||||||
return isIncludeMetadataEnabled; | ||||||
} | ||||||
|
||||||
public boolean isParallelFillReadSetEnabled() { | ||||||
return parallelFillReadSetEnabled; | ||||||
} | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might miss something, so just a question.
Don't we need these methods in
Delete
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need the
blind
option forDelete
because we always need the pre-read forDelete
. What do you think?