Add an extension point to collect uncommitted read-set and write-set #2271

komamitsu · 2024-10-03T12:07:34Z

Description

In the current implementation, it's not possible to develop an extension module injected via class loader that accesses the uncommitted read-set and write-set of a consensus commit transaction. This PR adds an extension point to collect uncommitted read-set and write-set to make it easy to develop extension modules to access those information.

Related issues and/or PRs

None

Changes made

Add SnapshotHandler that receives uncommitted Snapshot
Add CommitHandler.setSnapshotHandler()
Relax the access scopes of some information about ConsensusCommitManager and Snapshot

Checklist

The following is a best-effort checklist. If any items in this checklist are not applicable to this PR or are dependent on other, unmerged PRs, please still mark the checkboxes after you have read and understood each item.

I have commented my code, particularly in hard-to-understand areas.
I have updated the documentation to reflect the changes.
Any remaining open issues linked to this PR are documented and up-to-date (Jira, GitHub, etc.).
Tests (unit, integration, etc.) have been added for the changes.
My changes generate no new warnings.
Any dependent changes in other PRs have been merged and published.

Additional notes (optional)

None

Release notes

N/A

to make it inheritable

when SnapshotHandler is set in 2PC interface

komamitsu · 2024-10-03T12:10:56Z

core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitManager.java

  }

-  ConsensusCommitManager(DatabaseConfig databaseConfig) {
+  public ConsensusCommitManager(DatabaseConfig databaseConfig) {


To make it easy to inherit ConsensusCommitManager. This change is helpful to utilize the new extension point.

So, protected is more appropriate?

Suggested change

public ConsensusCommitManager(DatabaseConfig databaseConfig) {

protected ConsensusCommitManager(DatabaseConfig databaseConfig) {

Thanks! I confirmed it works. Addressed it in d2d1003

komamitsu · 2024-10-03T12:14:47Z

core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitUtils.java

+   * @param currentTxVersion The current `tx_version`, if it exists, or null otherwise.
+   * @return The next `tx_version`.
+   */
+  public static int getNextTxVersion(@Nullable Integer currentTxVersion) {


This logic how to determine the next Tx version may be reused by an extension module injected via class loader.

feeblefakie

Thank you!
Left some questions. PTAL!

feeblefakie · 2024-10-07T07:35:21Z

core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandler.java

      throw e;
    }

+    snapshotHandleFuture.ifPresent(SnapshotHandleFuture::get);


I might miss something, but is there any reason for using Future and defining SnapshotHandleFuture?
Can we just set Function or BiFunction like it does with setSnapshotHandler and invoke it here?

Also, I feel SnapshotHandler sounds too general.
Since it is invoked before the commit phase, we should probably name it something like BeforeCommitSnapshotHook.
What do you think?

@feeblefakie

Can we just set Function or BiFunction like it does with setSnapshotHandler and invoke it here?

Yeah, good point. I think using Function should also work.

I named the interface SnapshotHandle**Future** since I wanted to encourage setSnapshotHandler users to implement asynchronous handlers that don't block prepare() and validate(). Also, the reason why I wrapped Future with SnapshotHandleFuture is for maintainability considering future modifications just in case. But, I'm okay to go with simple Function.

Also, I feel SnapshotHandler sounds too general.

Ah, I agree. BeforeCommitSnapshotHook seems good. But it might sound like "validated" snapshots. How about PreparedSnapshotHook? Either is fine with me, though.

Thanks for the explanation.
Using Future for a non-blocking hook makes sense in this case since Snapshot won't be changed after the prepare phase, so I'm fine with it.
For the name, how about AfterPrepareSnapshotHook?
I'm also fine with BeforeCommitSnapshotHook and PreparedSnapshotHook.
I think you can decide.

@feeblefakie Thanks for the comment!

For the name, how about AfterPrepareSnapshotHook?
I'm also fine with BeforeCommitSnapshotHook and PreparedSnapshotHook.

Strictly speaking, snapshots are sent asynchronously before prepare() completes. So, I hesitate a bit to use the terms before or after that emphasizes the order. PreparedSnapshotHook might still imply that snapshots are already prepared, but I think it can be also interpreted that snapshots are in the process of being prepared, and relatively better. I'll go with PreparedSnapshotHook.

Addressed it in 0379139

feeblefakie

LGTM! Thank you!
I would like the class name to be changed, but any of the current options is fine with me.

feeblefakie · 2024-10-09T02:33:26Z

core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandler.java

      throw e;
    }

+    snapshotHandleFuture.ifPresent(SnapshotHandleFuture::get);


Thanks for the explanation.
Using Future for a non-blocking hook makes sense in this case since Snapshot won't be changed after the prepare phase, so I'm fine with it.
For the name, how about AfterPrepareSnapshotHook?
I'm also fine with BeforeCommitSnapshotHook and PreparedSnapshotHook.
I think you can decide.

brfrn169

Left several comments. Please take a look when you have time!

brfrn169 · 2024-10-09T05:00:55Z

core/src/main/java/com/scalar/db/transaction/consensuscommit/Snapshot.java

+   */
+  @SuppressFBWarnings(value = {"EI_EXPOSE_REP"})
+  public Map<Key, Optional<TransactionResult>> getReadSet() {
+    return readSet;


How about wrapping it with new HashMap() or ImmutableMap.copyOf()?

The snapshot hook mechanism introduced in this PR could impact the performance and memory consumption of applications that use ScalarDB. So, I think all the operations in a snapshot hook should be fast and low-memory-footprint as much as possible. As described in the method comment, it would be great if a snapshot hook can read those write set and read set without any coping entries.

I also thought the following iteration method, but it's too usage-specific.

public void iterateWriteSet(BiConsumer<Put, Optional<TransactionResult>> consumer) { for (Map.Entry<Key, Put> entry : writeSet.entrySet()) { Optional<TransactionResult> result = Optional.empty(); if (readSet.containsKey(entry.getKey())) { result = readSet.get(entry.getKey()); } consumer.accept(entry.getValue(), result); } }

I think using java.util.Collections#unmodifiableMap() is a reasonable option since it doesn't have its own table but delegates read operations to the original map. The performance impact should be negligible. What do you think?

public Map<Key, Optional<TransactionResult>> getReadSet() { return Collections.unmodifiableMap(readSet); }

I see. Sorry, I missed the method comment. That sounds reasonable.

I think using java.util.Collections#unmodifiableMap() is a reasonable option since it doesn't have its own table but delegates read operations to the original map. The performance impact should be negligible. What do you think?

Actually, I’m not sure about the differences in performance between Collections#unmodifiableMap(), new HashMap(), and ImmutableMap.copyOf(), so let's keep it as is for now.

By the way, we already have getter methods for writeSet and deleteSet, and they copy writeSet and deleteSet:

scalardb/core/src/main/java/com/scalar/db/transaction/consensuscommit/Snapshot.java

Lines 157 to 163 in 0379139

public List<Put> getPutsInWriteSet() {

return new ArrayList<>(writeSet.values());

}

public List<Delete> getDeletesInDeleteSet() {

return new ArrayList<>(deleteSet.values());

}

For the same reason, we probably don’t want to copy them. So could you please modify these methods as well and add similar method comments, even though it’s not directly related to this PR?

so let's keep it as is for now.

👍

As for getPutsInWriteSet() and getDeletesInDeleteSet(), they wrap the maps' values with ArrayList constructor, which also internally copies received values. So, I'll change the return type to Collection<Put> and Collection<Delete>, respectively.

Addressed them in 71b40a4

I think it might be better to do a simple benchmarking if we care about the performance.
ImmutableMap.copyOf actually tries to avoid copying if it is safe as described in the doc, so it might not cause much overhead.
https://guava.dev/releases/23.0/api/docs/com/google/common/collect/ImmutableMap.html#copyOf-java.util.Map-

Yeah, thanks for the heads-up. I micro-benchmarked yesterday with the following code. Sorry, I should've mentioned this benchmark.

package org.example; import com.google.common.collect.ImmutableMap; import java.util.Collections; import java.util.HashMap; import java.util.Map; public class IteratorTest { private static final Map<String, Integer> MAP = new HashMap<>(); static { MAP.put("one", 1); MAP.put("two", 2); MAP.put("three", 3); MAP.put("four", 4); MAP.put("five", 5); MAP.put("six", 6); MAP.put("seven", 7); MAP.put("eight", 8); MAP.put("nine", 9); MAP.put("ten", 10); } private static void testDirect() { Map<String, Integer> map = MAP; for (Map.Entry<String, Integer> entry : map.entrySet()) { entry.getKey(); entry.getValue(); } } private static void testUnmod() { Map<String, Integer> map = Collections.unmodifiableMap(MAP); for (Map.Entry<String, Integer> entry : map.entrySet()) { entry.getKey(); entry.getValue(); } } private static void testNewMap() { Map<String, Integer> map = new HashMap<>(MAP); for (Map.Entry<String, Integer> entry : map.entrySet()) { entry.getKey(); entry.getValue(); } } private static void testImmutable() { Map<String, Integer> map = ImmutableMap.copyOf(MAP); for (Map.Entry<String, Integer> entry : map.entrySet()) { entry.getKey(); entry.getValue(); } } public static void main(String[] args) { int n = 1000000; for (int i = 0; i < n; i++) { testDirect(); } { long start = System.currentTimeMillis(); for (int i = 0; i < n; i++) { testDirect(); } System.out.println("Direct: duration=" + (System.currentTimeMillis() - start)); } for (int i = 0; i < n; i++) { testUnmod(); } { long start = System.currentTimeMillis(); for (int i = 0; i < n; i++) { testUnmod(); } System.out.println("UnmodiMap: duration=" + (System.currentTimeMillis() - start)); } for (int i = 0; i < n; i++) { testNewMap(); } { long start = System.currentTimeMillis(); for (int i = 0; i < n; i++) { testNewMap(); } System.out.println("NewMap: duration=" + (System.currentTimeMillis() - start)); } for (int i = 0; i < n; i++) { testImmutable(); } { long start = System.currentTimeMillis(); for (int i = 0; i < n; i++) { testImmutable(); } System.out.println("Immutable: duration=" + (System.currentTimeMillis() - start)); } } }

The result was:

> Task :IteratorTest.main() Direct: duration=20 UnmodiMap: duration=21 NewMap: duration=89 Immutable: duration=130

As for the Javadoc, I also looked into the implementation. It doesn't copy only when the original map is ImmutableMap instance. In the case of Snapshot. the original maps are HashMap and I think they'll be copied.

brfrn169 · 2024-10-09T05:01:03Z

core/src/main/java/com/scalar/db/transaction/consensuscommit/Snapshot.java

+   */
+  @SuppressFBWarnings(value = {"EI_EXPOSE_REP"})
+  public Map<Key, Put> getWriteSet() {
+    return writeSet;


brfrn169 · 2024-10-09T05:01:10Z

core/src/main/java/com/scalar/db/transaction/consensuscommit/Snapshot.java

+   */
+  @SuppressFBWarnings(value = {"EI_EXPOSE_REP"})
+  public Map<Key, Delete> getDeleteSet() {
+    return deleteSet;


brfrn169 · 2024-10-09T05:11:29Z

core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitManager.java

  }

-  ConsensusCommitManager(DatabaseConfig databaseConfig) {
+  public ConsensusCommitManager(DatabaseConfig databaseConfig) {


So, protected is more appropriate?

Suggested change

public ConsensusCommitManager(DatabaseConfig databaseConfig) {

protected ConsensusCommitManager(DatabaseConfig databaseConfig) {

brfrn169 · 2024-10-09T05:12:46Z

core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandler.java

+    Optional<PreparedSnapshotHookFuture> snapshotHookFuture;
    try {
-      prepare(snapshot);
+      snapshotHookFuture = prepare(snapshot);


Maybe we can move the snapshotHandler logic outside the prepare() method as follows?

Suggested change

snapshotHookFuture = prepare(snapshot);

snapshotHandleFuture = handleSnapshotBeforePreparation(snapshot);

prepare(snapshot);

What do you think?

Ah, good point! I missed the improvement after we decided to use Snapshot itself... I'll update the PR.

Addressed them in 067d69d.

@brfrn169 I also noticed the original PR didn't properly handle errors when invoking the snapshot hook and waiting the snapshot future, resulting in throwing non-CommitException and resource leaks. I enhanced the error handling in the commit. PTAL!

brfrn169 · 2024-10-09T05:29:54Z

core/src/main/java/com/scalar/db/transaction/consensuscommit/PreparedSnapshotHook.java

@@ -0,0 +1,10 @@
+package com.scalar.db.transaction.consensuscommit;
+
+public interface PreparedSnapshotHook {


PreparedSnapshotHook sounds like a hook called after preparation to me. Actually, the state of the snapshot can change after preparation.

scalardb/core/src/main/java/com/scalar/db/transaction/consensuscommit/Snapshot.java

Line 459 in 0379139

writeSet.put(entry.getKey(), put);

Maybe BeforePreparationSnapshotHook would be a better name?

In the future, we might add something like BeforeValidationSnapshotHook or BeforeCommitSnapshotHook.

What do you think?

PreparedSnapshotHook sounds like a hook called after preparation to me.

Indeed, I have the same concern slightly. #2271 (comment)

It's possible that the prepared records are asynchronously written before a snapshot is sent. So, I was also wondering if BeforePreparationSnapshotHook-ish name was reasonable or not. But, I think it can mean the snapshot is sent before the prepare phase operation. So, I think it's okay. I'll change it to BeforePreparationSnapshotHook.

Addressed it in 0fe41c9

As Toshi said, snapshot states might be changed in the case of extra-write. So, we shouldn't trigger the hook asynchronously?

@feeblefakie

Ah, you're right... I just remembered that @brfrn169 and I discussed this case and planed not to support the snapshot hook feature when extra-write is enabled, since extra-write would be sunset-ed (so, this PR should've checked the configuration...).

Triggering the hook after preparation might increase the total latency. So, I think the invocation of the hook should be asynchronous without waiting for the completion of prepare phase operation, although it might increase the possibility of transactions that abort in prepare phase being handled by the snapshot handler to some extent. What do you think?

So, do you think we should give up passing a snapshot and just passes the immutable read/write-sets like this?

Yes, that’s what I had in mind. Thanks.

Done

@brfrn169 I noticed new ArrayList(map.entrySet()) is twice faster than returning new HashMap(map). So, I added the method List<Entry<Key, Put>> getKeysAndPutsInWriteSet() instead of Map<Key, Put> getWriteSetMap(). I think the new method is very similar to the existing method List<Put> getPutsInWriteSet(). Probably we can replace the usage of List<Put> getPutsInWriteSet() with the new method. What do you think?

As for extra-write, yeah I'll add a validation for that in this PR later.

Done

On second thought, whether to prohibit EXTRA WRITE depends on the usage of write sets. On the above discussions, I was only thinking of the use case of replication. But, write set captured before prepare might work with EXTRA WRITE in other use cases. So, I think a validation to prohibit using EXTRA WRITE should be handled by a plug-in side not in scalardb:core. Probably I should revert 8f0817d ...

So, do you think we should give up passing a snapshot and just passes the immutable read/write-sets like this?

Yes, that’s what I had in mind. Thanks.

Done

Oops. What I did was not along with this idea. I'll address this...

Sorry for the late fix. I updated the PR to pass ReadWriteSet instead of Snapshot itself in d540b4b.

On second thought, whether to prohibit EXTRA WRITE depends on the usage of write sets. On the above discussions, I was only thinking of the use case of replication. But, write set captured before prepare might work with EXTRA WRITE in other use cases. So, I think a validation to prohibit using EXTRA WRITE should be handled by a plug-in side not in scalardb:core. Probably I should revert 8f0817d ...

Reverted

brfrn169 · 2024-10-10T06:49:48Z

core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandler.java

  private final TransactionTableMetadataManager tableMetadataManager;
  private final ParallelExecutor parallelExecutor;

+  @LazyInit @Nullable private BeforePreparationSnapshotHook beforePreparationSnapshotHook;


It's a minor thing, but I thought @LazyInit also implied that the field was nullable. Should we add @Nullable alongside @LazyInit as well?

I thought @LazyInit also implied that the field was nullable.

Does it point that the field can be null until it's lazily initialized? If so, I think only @LazyInit may be enough. @Nullable is added since the field can be permanently null if no snapshot hook is set. Does it answer your question?

That makes sense. Thanks!

brfrn169 · 2024-10-10T06:57:00Z

core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandler.java

+    try {
+      if (beforePreparationSnapshotHook == null) {
+        return Optional.empty();
+      }
+
+      return Optional.of(beforePreparationSnapshotHook.handle(tableMetadataManager, snapshot));
+    } catch (Exception e) {


Small nit:

Suggested change

try {

if (beforePreparationSnapshotHook == null) {

return Optional.empty();

}

return Optional.of(beforePreparationSnapshotHook.handle(tableMetadataManager, snapshot));

} catch (Exception e) {

if (beforePreparationSnapshotHook == null) {

return Optional.empty();

}

try {

return Optional.of(beforePreparationSnapshotHook.handle(tableMetadataManager, snapshot));

} catch (Exception e) {

brfrn169 · 2024-10-10T06:57:36Z

core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandler.java

+      // TODO: This method is actually a part of preparation phase. But the callback method name
+      //       `onPrepareFailure()` should be renamed to more reasonable one.
+      onPrepareFailure(snapshot);
+      throw new CommitException(e.getMessage(), e, snapshot.getId());


I'm wondering if we should define a new error message for this as follows:

Suggested change

throw new CommitException(e.getMessage(), e, snapshot.getId());

throw new CommitException(

CoreError.HANDLING_BEFORE_PREPARATION_SNAPSHOT_HOOK_FAILED.buildMessage(e.getMessage()),

e,

snapshot.getId());

brfrn169 · 2024-10-10T07:00:23Z

core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandler.java

+      // TODO: This method is actually a part of validation phase. But the callback method name
+      //       `onValidateFailure()` should be renamed to more reasonable one.
+      onValidateFailure(snapshot);
+      throw new CommitException(e.getMessage(), e, snapshot.getId());


Ditto.

Suggested change

throw new CommitException(e.getMessage(), e, snapshot.getId());

throw new CommitException(

CoreError.WAITING_BEFORE_PREPARATION_SNAPSHOT_HOOK_FAILED.buildMessage(e.getMessage()),

e,

snapshot.getId());

Good catch! I'll define and use the new error code.

brfrn169 · 2024-10-10T15:33:41Z

core/src/main/java/com/scalar/db/transaction/consensuscommit/BeforePreparationSnapshotHook.java

+package com.scalar.db.transaction.consensuscommit;
+
+public interface BeforePreparationSnapshotHook {
+  BeforePreparationSnapshotHookFuture handle(


Sorry, one more thing. Why not use the Java Future as follows?

Suggested change

BeforePreparationSnapshotHookFuture handle(

Future<Void> handle(

I first thought using the custom future would be helpful if we add a new API like cancel() to the future. But it also requires some changes in CommitHandler to call new APIs. Since we can postpone implementing the custom future until we actually need such new APIs, I'll go ahead and merge your suggestion to simplify things for now.

This reverts commit 71b40a4.

instead of Snapshot

This reverts commit 5b77134.

komamitsu · 2024-10-16T06:40:12Z

@feeblefakie JFYI: I added the following changes after you approved this PR, based on some second thoughts and discussion with @brfrn169. Feel free to share your concerns or thoughts.

Renamed PreparedSnapshotHook to BeforePreparationSnapshotHook
- Reason: Prepared still sounds "after-prepare-phase"
Made BeforePreparationSnapshotHook.handle() simply return Future<Void> for now
- Reason: I initially used PreparedSnapshotHookFuture since I had considered the potential extendability of the return value. But, we would need to modify CommitHandler as well if we add a new API to the return Future-ish value. Using PreparedSnapshotHookFuture isn't so useful at the moment.
Passed immutable Snapshot.ReadWriteSet instead of mutable Snapshot itself to the snapshot hooks.
- Reason: For safety considering future changes.

@brfrn169 I updated this PR. PTAL when you have a chance.

brfrn169

Left a minor comment. Other than that, LGTM! Thank you!

brfrn169 · 2024-10-16T06:54:41Z

core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandler.java

+          CoreError.HANDLING_BEFORE_PREPARATION_SNAPSHOT_HOOK_FAILED.buildMessage(e.getMessage()),
+          e,


When catching ExecutionException, the actual exception is wrapped by ExecutionException, so maybe we should get the cause exception by the getCause() method in that case?

Future (== FutureTask) wraps an exception occurs in get() with new ExecutionException((Throwable)cause). new Throwable(Throwable cause) sets detailedMessage to cause.toString(). So, I think it's okay in this case.

ExecutorService executorService = Executors.newCachedThreadPool(); Future<Object> future = executorService.submit(() -> { throw new RuntimeException("This is the original exception"); }); executorService.shutdown(); try { future.get(); } catch (Exception e) { System.out.println("Message from the exception: " + e.getMessage()); System.out.println("Message from the cause: " + e.getCause()); }

Message from the exception: java.lang.RuntimeException: This is the original exception Message from the cause: java.lang.RuntimeException: This is the original exception

Torch3333

LGTM, thank you!

…2271)

komamitsu added 6 commits October 1, 2024 16:28

Add WriteOperationsHandler interface

5139983

Change WriteOperationsHandler to SnapshotHandler

635e1ec

Make ConsensusCommitManager.commit protected

d8b62fc

Add unit tests

33081d8

Make a constructor of ConsensusCommitManager public

b04440e

to make it inheritable

Throw an AssertionError

d36a300

when SnapshotHandler is set in 2PC interface

komamitsu commented Oct 3, 2024

View reviewed changes

komamitsu requested review from Torch3333, brfrn169 and feeblefakie October 3, 2024 12:15

komamitsu self-assigned this Oct 3, 2024

komamitsu added the improvement label Oct 3, 2024

Address spotbugs warnings

cfe0790

feeblefakie reviewed Oct 7, 2024

View reviewed changes

komamitsu requested a review from feeblefakie October 8, 2024 01:41

feeblefakie approved these changes Oct 9, 2024

View reviewed changes

Rename SnapshotHandler to PreparedSnapshotHook

0379139

brfrn169 reviewed Oct 9, 2024

View reviewed changes

komamitsu added 5 commits October 9, 2024 14:49

Change the scope of constructor to protected

d2d1003

Refactor & enhance error handling in CommitHandler

067d69d

Refactor some Snapshot methods for internal use

71b40a4

Rename PreparedSnapshotHook to BeforePreparationSnapshotHook

0fe41c9

Fix CommitHandlerTest

bdd43db

komamitsu requested a review from brfrn169 October 9, 2024 11:09

brfrn169 reviewed Oct 10, 2024

View reviewed changes

komamitsu added 2 commits October 10, 2024 16:54

Refactoring

964f5fa

Define and use a new error code

2e5db19

komamitsu requested a review from brfrn169 October 10, 2024 10:22

brfrn169 reviewed Oct 10, 2024

View reviewed changes

komamitsu added 5 commits October 15, 2024 13:48

Revert "Refactor some Snapshot methods for internal use"

b1d4320

This reverts commit 71b40a4.

Update newly added methods in Snapshot

8f0817d

Use Future<Void>

657558c

Prohibit snapshot hook with EXTRA_WRITE

5b77134

Fix unit test

d63c4e0

komamitsu requested a review from brfrn169 October 15, 2024 08:26

komamitsu added 2 commits October 16, 2024 14:44

Pass ReadWriteSets to snapshot hook

d540b4b

instead of Snapshot

Revert "Prohibit snapshot hook with EXTRA_WRITE"

882e476

This reverts commit 5b77134.

brfrn169 approved these changes Oct 16, 2024

View reviewed changes

Torch3333 approved these changes Oct 18, 2024

View reviewed changes

Merge branch 'master' into add-snapshot-handler

cffecbc

komamitsu merged commit 3b260bc into master Oct 18, 2024
46 checks passed

komamitsu deleted the add-snapshot-handler branch October 18, 2024 04:01

feeblefakie pushed a commit that referenced this pull request Oct 18, 2024

Add an extension point to collect uncommitted read-set and write-set (#…

b29e37b

…2271)

feeblefakie mentioned this pull request Oct 18, 2024

Backport to branch(3) : Add an extension point to collect uncommitted read-set and write-set #2287

Merged

	public ConsensusCommitManager(DatabaseConfig databaseConfig) {
	protected ConsensusCommitManager(DatabaseConfig databaseConfig) {

	public List<Put> getPutsInWriteSet() {
	return new ArrayList<>(writeSet.values());
	}

	public List<Delete> getDeletesInDeleteSet() {
	return new ArrayList<>(deleteSet.values());
	}

	snapshotHookFuture = prepare(snapshot);
	snapshotHandleFuture = handleSnapshotBeforePreparation(snapshot);
	prepare(snapshot);

		@@ -0,0 +1,10 @@
		package com.scalar.db.transaction.consensuscommit;

		public interface PreparedSnapshotHook {

	BeforePreparationSnapshotHookFuture handle(
	Future<Void> handle(

		CoreError.HANDLING_BEFORE_PREPARATION_SNAPSHOT_HOOK_FAILED.buildMessage(e.getMessage()),
		e,

Add an extension point to collect uncommitted read-set and write-set #2271

Add an extension point to collect uncommitted read-set and write-set #2271

Uh oh!

Conversation

komamitsu commented Oct 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues and/or PRs

Changes made

Checklist

Additional notes (optional)

Release notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

feeblefakie left a comment

Choose a reason for hiding this comment

Uh oh!

feeblefakie Oct 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

komamitsu Oct 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

komamitsu Oct 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

feeblefakie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brfrn169 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

komamitsu Oct 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brfrn169 Oct 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

komamitsu commented Oct 3, 2024 •

edited

Loading

feeblefakie Oct 7, 2024 •

edited

Loading

komamitsu Oct 7, 2024 •

edited

Loading

komamitsu Oct 9, 2024 •

edited

Loading

komamitsu Oct 9, 2024 •

edited

Loading

brfrn169 Oct 9, 2024 •

edited

Loading

komamitsu Oct 10, 2024 •

edited

Loading