-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Accord: Add Rebootstrap and unsafe Bootstrap #4387
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
@@ -1,4 +1,4 @@ | ||
[submodule "modules/accord"] | ||
path = modules/accord | ||
url = https://github.com/apache/cassandra-accord.git | ||
branch = trunk | ||
url = https://github.com/belliottsmith/cassandra-accord.git | ||
branch = rebootstrap-sq |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,7 @@ | |
import accord.primitives.Range; | ||
import accord.primitives.Ranges; | ||
import accord.primitives.SyncPoint; | ||
import accord.utils.UnhandledEnum; | ||
import org.apache.cassandra.db.ColumnFamilyStore; | ||
import org.apache.cassandra.db.memtable.Memtable; | ||
import org.apache.cassandra.schema.Schema; | ||
|
@@ -40,14 +41,6 @@ public class AccordDataStore implements DataStore | |
private static final Logger logger = LoggerFactory.getLogger(AccordDataStore.class); | ||
enum FlushListenerKey { KEY } | ||
|
||
@Override | ||
public FetchResult fetch(Node node, SafeCommandStore safeStore, Ranges ranges, SyncPoint syncPoint, FetchRanges callback) | ||
{ | ||
AccordFetchCoordinator coordinator = new AccordFetchCoordinator(node, ranges, syncPoint, callback, safeStore.commandStore()); | ||
coordinator.start(); | ||
return coordinator.result(); | ||
} | ||
|
||
/** | ||
* Ensures data for the intersecting ranges is flushed to sstable before calling back with reportOnSuccess. | ||
* This is used to gate journal cleanup, since we skip the CommitLog for applying to the data table. | ||
|
@@ -95,4 +88,23 @@ public void ensureDurable(CommandStore commandStore, Ranges ranges, RedundantBef | |
prev = cfs; | ||
} | ||
} | ||
|
||
@Override | ||
public FetchResult fetch(Node node, SafeCommandStore safeStore, Ranges ranges, SyncPoint syncPoint, FetchRanges callback, FetchKind kind) | ||
{ | ||
switch (kind) | ||
{ | ||
default: throw new UnhandledEnum(kind); | ||
case Image: | ||
{ | ||
AccordFetchCoordinator coordinator = new AccordFetchCoordinator(node, ranges, syncPoint, callback, safeStore.commandStore()); | ||
coordinator.start(); | ||
return coordinator.result(); | ||
} | ||
case Sync: | ||
{ | ||
throw new UnsupportedOperationException(); | ||
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. Is this implemented in some other way? I believe I had an implementation of repair plugged in here. 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 logic is also disabled for now, I plan to introduce it in a follow up patch. 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. Just for my understanding: this patch is then just to make Accord compile? 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. No, it also supports bootstrapping a node that was previously bootstrapped. That said, let me see about at least restoring this part of the patch, and we can address the journal replay issue in a follow up. 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. Either way: I am also completely fine with doing this in a follow-up patch. |
||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -585,7 +585,7 @@ public void forEach(Consumer<JournalKey> consumer) | |
|
||
@SuppressWarnings("unchecked") | ||
@Override | ||
public void replay(CommandStores commandStores) | ||
public boolean replay(CommandStores commandStores) | ||
{ | ||
// TODO (expected): make the parallelisms configurable | ||
// Replay is performed in parallel, where at most X commands can be in flight, accross at most Y commands stores. | ||
|
@@ -716,6 +716,7 @@ public void close() | |
|
||
++cur; | ||
} | ||
return true; | ||
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. should we return false on corruption? 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 logic is essentially disabled for now as we discussed, I will introduce it more fully in a follow up patch. 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. I can not recall any conversation about this logic being disabled. Searched messages quickly and couldn't find anything either. Maybe I misunderstood when it got mentioned. |
||
} | ||
catch (Throwable t) | ||
{ | ||
|
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'm sure you had it in mind, but just for completeness, reminding to switch branch.