Skip to content

Commit

Permalink
refactor(xlineapi): improve is_conflict
Browse files Browse the repository at this point in the history 
Signed-off-by: lxl66566 <[email protected]>

refactor(xlineapi): complete refactor

Signed-off-by: lxl66566 <[email protected]>
  • Loading branch information
@lxl66566
lxl66566 committed Aug 2, 2024
1 parent eb7da44 commit cdb5296
Show file tree
Hide file tree
Showing 3 changed files with 255 additions and 181 deletions.
137 changes: 137 additions & 0 deletions crates/xlineapi/src/classifier.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
use super::RequestWrapper;

/// Backend store of request
#[allow(missing_docs)]
#[derive(Debug, PartialEq, Eq)]
pub enum RequestBackend {
Kv,
Auth,
Lease,
Alarm,
}

impl From<&RequestWrapper> for RequestBackend {
fn from(value: &RequestWrapper) -> Self {
match *value {
RequestWrapper::PutRequest(_)
| RequestWrapper::RangeRequest(_)
| RequestWrapper::DeleteRangeRequest(_)
| RequestWrapper::TxnRequest(_)
| RequestWrapper::CompactionRequest(_) => RequestBackend::Kv,
RequestWrapper::AuthEnableRequest(_)
| RequestWrapper::AuthDisableRequest(_)
| RequestWrapper::AuthStatusRequest(_)
| RequestWrapper::AuthRoleAddRequest(_)
| RequestWrapper::AuthRoleDeleteRequest(_)
| RequestWrapper::AuthRoleGetRequest(_)
| RequestWrapper::AuthRoleGrantPermissionRequest(_)
| RequestWrapper::AuthRoleListRequest(_)
| RequestWrapper::AuthRoleRevokePermissionRequest(_)
| RequestWrapper::AuthUserAddRequest(_)
| RequestWrapper::AuthUserChangePasswordRequest(_)
| RequestWrapper::AuthUserDeleteRequest(_)
| RequestWrapper::AuthUserGetRequest(_)
| RequestWrapper::AuthUserGrantRoleRequest(_)
| RequestWrapper::AuthUserListRequest(_)
| RequestWrapper::AuthUserRevokeRoleRequest(_)
| RequestWrapper::AuthenticateRequest(_) => RequestBackend::Auth,
RequestWrapper::LeaseGrantRequest(_)
| RequestWrapper::LeaseRevokeRequest(_)
| RequestWrapper::LeaseLeasesRequest(_) => RequestBackend::Lease,
RequestWrapper::AlarmRequest(_) => RequestBackend::Alarm,
}
}
}

/// Type of request. This is the extending of [`RequestBackend`].
#[allow(missing_docs)]
#[derive(Debug, PartialEq, Eq)]
pub enum RequestType {
Put,
Compaction,
Txn,
Range,
Auth,
Lease,
Alarm,
}

impl From<&RequestWrapper> for RequestType {
fn from(value: &RequestWrapper) -> Self {
match *value {
RequestWrapper::PutRequest(_) => RequestType::Put,
RequestWrapper::RangeRequest(_) | RequestWrapper::DeleteRangeRequest(_) => {
RequestType::Range
}
RequestWrapper::TxnRequest(_) => RequestType::Txn,
RequestWrapper::CompactionRequest(_) => RequestType::Compaction,
RequestWrapper::AuthEnableRequest(_)
| RequestWrapper::AuthDisableRequest(_)
| RequestWrapper::AuthStatusRequest(_)
| RequestWrapper::AuthRoleAddRequest(_)
| RequestWrapper::AuthRoleDeleteRequest(_)
| RequestWrapper::AuthRoleGetRequest(_)
| RequestWrapper::AuthRoleGrantPermissionRequest(_)
| RequestWrapper::AuthRoleListRequest(_)
| RequestWrapper::AuthRoleRevokePermissionRequest(_)
| RequestWrapper::AuthUserAddRequest(_)
| RequestWrapper::AuthUserChangePasswordRequest(_)
| RequestWrapper::AuthUserDeleteRequest(_)
| RequestWrapper::AuthUserGetRequest(_)
| RequestWrapper::AuthUserGrantRoleRequest(_)
| RequestWrapper::AuthUserListRequest(_)
| RequestWrapper::AuthUserRevokeRoleRequest(_)
| RequestWrapper::AuthenticateRequest(_) => RequestType::Auth,
RequestWrapper::LeaseGrantRequest(_)
| RequestWrapper::LeaseRevokeRequest(_)
| RequestWrapper::LeaseLeasesRequest(_) => RequestType::Lease,
RequestWrapper::AlarmRequest(_) => RequestType::Alarm,
}
}
}

/// indicates if the request is readonly or write
#[derive(Debug, PartialEq, Eq)]
pub enum RequestRw {
/// Read only request
Read,
/// Write request.
///
/// NOTE: A `TxnRequest` or a `DeleteRangeRequest` might be read-only, but we
/// assume they will mutate the state machine to simplify the implementation.
Write,
}

impl From<&RequestWrapper> for RequestRw {
fn from(value: &RequestWrapper) -> Self {
match *value {
RequestWrapper::RangeRequest(_)
| RequestWrapper::AuthStatusRequest(_)
| RequestWrapper::AuthRoleGetRequest(_)
| RequestWrapper::AuthRoleListRequest(_)
| RequestWrapper::AuthUserGetRequest(_)
| RequestWrapper::AuthUserListRequest(_)
| RequestWrapper::LeaseLeasesRequest(_) => Self::Read,

RequestWrapper::PutRequest(_)
| RequestWrapper::DeleteRangeRequest(_)
| RequestWrapper::TxnRequest(_)
| RequestWrapper::CompactionRequest(_)
| RequestWrapper::AuthEnableRequest(_)
| RequestWrapper::AuthDisableRequest(_)
| RequestWrapper::AuthRoleAddRequest(_)
| RequestWrapper::AuthRoleDeleteRequest(_)
| RequestWrapper::AuthRoleGrantPermissionRequest(_)
| RequestWrapper::AuthRoleRevokePermissionRequest(_)
| RequestWrapper::AuthUserAddRequest(_)
| RequestWrapper::AuthUserChangePasswordRequest(_)
| RequestWrapper::AuthUserDeleteRequest(_)
| RequestWrapper::AuthUserGrantRoleRequest(_)
| RequestWrapper::AuthUserRevokeRoleRequest(_)
| RequestWrapper::AuthenticateRequest(_)
| RequestWrapper::LeaseGrantRequest(_)
| RequestWrapper::LeaseRevokeRequest(_)
| RequestWrapper::AlarmRequest(_) => Self::Write,
}
}
}
170 changes: 112 additions & 58 deletions crates/xlineapi/src/command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ use prost::Message;
use serde::{Deserialize, Serialize};

use crate::{
execute_error::ExecuteError, AuthInfo, PbCommand, PbCommandResponse, PbKeyRange,
PbSyncResponse, RequestWrapper, ResponseWrapper,
classifier::{RequestBackend, RequestRw, RequestType},
execute_error::ExecuteError,
AuthInfo, PbCommand, PbCommandResponse, PbKeyRange, PbSyncResponse, RequestWrapper,
ResponseWrapper,
};

/// The curp client trait object on the command of xline
Expand Down Expand Up @@ -221,67 +223,119 @@ pub struct Command {
auth_info: Option<AuthInfo>,
}

impl ConflictCheck for Command {
#[inline]
fn is_conflict(&self, other: &Self) -> bool {
let this_req = &self.request;
let other_req = &other.request;
// auth read request will not conflict with any request except the auth write request
if (this_req.is_auth_read_request() && other_req.is_auth_read_request())
|| (this_req.is_kv_request() && other_req.is_auth_read_request())
|| (this_req.is_auth_read_request() && other_req.is_kv_request())
{
return false;
}
// any two requests that don't meet the above conditions will conflict with each other
// because the auth write request will make all previous token invalid
if this_req.is_auth_request()
|| other_req.is_auth_request()
|| this_req.is_alarm_request()
|| other_req.is_alarm_request()
{
return true;
}
/// Match all Classifiers seperated by `&`

Check warning on line 226 in crates/xlineapi/src/command.rs

View workflow job for this annotation

GitHub Actions / Spell Check 

"seperated" should be "separated".
///
/// # Returns
///
/// `Fn(x) -> bool` indicates the match result.
///
/// # Example
///
/// ```ignore
/// match_all!(Class1::Tag1 & Class2::Tag2)(x)
/// ```
macro_rules! match_all {
($($cls:ident :: $tag:ident)&*) => {
|_x| $(matches!($cls::from(_x), $cls::$tag))&&*
};
}
pub(crate) use match_all; // used by lib.rs

// Lease leases request is conflict with Lease grant and revoke requests
if (this_req.is_lease_read_request() && other_req.is_lease_write_request())
|| (this_req.is_lease_write_request() && other_req.is_lease_read_request())
{
return true;
/// swapable match, returns a `Fn(x, y) -> bool` indicates the match result.
///
/// # Returns
///
/// Returns `Fn(x, y) -> true` if *x match Class1 and y match Class2*, or *x match Class2 and y match Class1*.
macro_rules! swap_match {

($($cls1:ident::$tag1:ident)&*, _) => {{
|_x, _| match_all!($($cls1::$tag1)&*)(_x)
}};
($($cls1:ident :: $tag1:ident)&*, $($cls2:ident :: $tag2:ident)&*) => {
|_x, _y| {
(match_all!($($cls1::$tag1)&*)(_x) && match_all!($($cls2::$tag2)&*)(_y)) || (
match_all!($($cls2::$tag2)&*)(_x) && match_all!($($cls1::$tag1)&*)(_y)
)
}
};
}

if this_req.is_compaction_request() && other_req.is_compaction_request() {
return true;
}
/// swapable map, to swappable match two `RequestWrapper`, extract the inner request
/// and calculate the inners into an `Option<T>`.
/// This can be only used in the same enum and different variant.
///
/// # Returns
///
/// `Fn(x, y) -> Option<bool>` indicates the swap_map result.
macro_rules! swap_map {
($cls1:ident :: $tag1:ident, $cls2:ident :: $tag2:ident, |$x:ident, $y:ident| $body:expr) => {
(|_self, _other| match (_self, _other) {
(&$cls1::$tag1(ref $x), &$cls2::$tag2(ref $y))
| (&$cls2::$tag2(ref $y), &$cls1::$tag1(ref $x)) => Some($body),
_ => None,
})
};
}

if (this_req.is_txn_request() && other_req.is_compaction_request())
|| (this_req.is_compaction_request() && other_req.is_txn_request())
{
match (this_req, other_req) {
(
&RequestWrapper::CompactionRequest(ref com_req),
&RequestWrapper::TxnRequest(ref txn_req),
)
| (
&RequestWrapper::TxnRequest(ref txn_req),
&RequestWrapper::CompactionRequest(ref com_req),
) => {
let target_revision = com_req.revision;
return txn_req.is_conflict_with_rev(target_revision)
}
_ => unreachable!("The request must be either a transaction or a compaction request! \nthis_req = {this_req:?} \nother_req = {other_req:?}")
}
/// Conflict check, contains the whole `match` statement
///
/// # Returns
///
/// `Fn(x, y) -> Option<bool>` indicates the conflict result.
macro_rules! is_conflict {
($(($body:expr, $($cls1:ident :: $tag1:ident)&*, $($pat:tt)*)),*) => {
|_self, _other| match (_self, _other) {
$((x, y) if swap_match!($($cls1::$tag1)&*, $($pat)*)(x, y) => Some($body),)*
_ => None,
}
};
}

let this_lease_ids = this_req.leases().into_iter().collect::<HashSet<_>>();
let other_lease_ids = other_req.leases().into_iter().collect::<HashSet<_>>();
let lease_conflict = !this_lease_ids.is_disjoint(&other_lease_ids);
let key_conflict = self
.keys()
.iter()
.cartesian_product(other.keys().iter())
.any(|(k1, k2)| k1.is_conflicted(k2));
lease_conflict || key_conflict
impl ConflictCheck for Command {
#[inline]
fn is_conflict(&self, other: &Self) -> bool {
let t_req = &self.request;
let o_req = &other.request;
is_conflict!(
// auth read request will not conflict with any request except the auth write request
(
true,
RequestBackend::Auth & RequestRw::Read,
RequestBackend::Auth & RequestRw::Write
),
(false, RequestBackend::Auth & RequestRw::Read, _),
// any two requests that don't meet the above conditions will conflict with each other
// because the auth write request will make all previous token invalid
(true, RequestBackend::Auth & RequestRw::Write, _),
(true, RequestBackend::Alarm, _),
// Lease leases request is conflict with Lease grant and revoke requests
(
true,
RequestBackend::Lease & RequestRw::Read,
RequestBackend::Lease & RequestRw::Write
),
(true, RequestType::Compaction, RequestType::Compaction)
)(t_req, o_req)
.or_else(|| {
swap_map!(
RequestWrapper::TxnRequest,
RequestWrapper::CompactionRequest,
|x, y| x.is_conflict_with_rev(y.revision)
)(t_req, o_req)
})
// the fallback map
.or_else(|| {
let this_lease_ids = t_req.leases().into_iter().collect::<HashSet<_>>();
let other_lease_ids = o_req.leases().into_iter().collect::<HashSet<_>>();
let lease_conflict = !this_lease_ids.is_disjoint(&other_lease_ids);
let key_conflict = self
.keys()
.iter()
.cartesian_product(other.keys().iter())
.any(|(k1, k2)| k1.is_conflict(k2));
Some(lease_conflict || key_conflict)
})
.unwrap_or_default()
}
}

Expand Down Expand Up @@ -476,7 +530,7 @@ impl CurpCommand for Command {

#[inline]
fn is_read_only(&self) -> bool {
self.request().is_read_only()
match_all!(RequestRw::Read)(self.request())
}
}

Expand Down
Loading

0 comments on commit cdb5296

Please sign in to comment.