Skip to content

Commit

Permalink
refactor: access KVMeta with methods instead of field
Browse files Browse the repository at this point in the history 
Make `KVMeta.expire_at` a private field for future refactoring.
Update accessing to it with via methods.

Several methods are added to `KVMeta` and `SeqValue`:

- `eval_expire_at_ms()` evaluate and return a absolute expire time in millisecond. The returned value is set to `u64::MAX` if not expire is set.
- `get_expire_at_ms()` return an `Option` of absolute expire time in millisecond.
  • Loading branch information
@drmingdrmer
drmingdrmer committed Dec 14, 2023
1 parent 457baeb commit 0e0f62f
Show file tree
Hide file tree
Showing 19 changed files with 126 additions and 241 deletions.
4 changes: 1 addition & 3 deletions src/binaries/meta/kvapi.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@ impl KvApiCommand {
let req = UpsertKVReq::update(config.key[0].as_str(), config.value.as_bytes());

let req = if let Some(expire_after) = config.expire_after {
req.with(KVMeta {
expire_at: Some(SeqV::<()>::now_ms() / 1000 + expire_after),
})
req.with(KVMeta::new_expire(SeqV::<()>::now_sec() + expire_after))
} else {
req
};
Expand Down
4 changes: 1 addition & 3 deletions src/meta/api/src/background_api_impl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,7 @@ impl<KV: kvapi::KVApi<Error = MetaError>> BackgroundApi for KV {
name_key.to_string_key().as_str(),
Any,
Operation::Update(serialize_struct(&meta)?),
Some(KVMeta {
expire_at: Some(req.expire_at),
}),
Some(KVMeta::new_expire(req.expire_at)),
))
.await?;
// confirm a successful update
Expand Down
40 changes: 9 additions & 31 deletions src/meta/kvapi/src/kvapi/test_suite.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,7 @@ impl kvapi::TestSuite {
.as_secs();

let _res = kv
.upsert_kv(UpsertKVReq::update("k1", b"v1").with(KVMeta {
expire_at: Some(now + 2),
}))
.upsert_kv(UpsertKVReq::update("k1", b"v1").with(KVMeta::new_expire(now + 2)))
.await?;
// dbg!("upsert non expired k1", _res);

Expand Down Expand Up @@ -288,9 +286,7 @@ impl kvapi::TestSuite {
.upsert_kv(
UpsertKVReq::update("k1", b"v1")
.with(MatchSeq::Exact(0))
.with(KVMeta {
expire_at: Some(now - 1),
}),
.with(KVMeta::new_expire(now - 1)),
)
.await?;
// dbg!("update expired k1", _res);
Expand All @@ -299,9 +295,7 @@ impl kvapi::TestSuite {
.upsert_kv(
UpsertKVReq::update("k2", b"v2")
.with(MatchSeq::Exact(0))
.with(KVMeta {
expire_at: Some(now + 10),
}),
.with(KVMeta::new_expire(now + 10)),
)
.await?;
// dbg!("update non expired k2", _res);
Expand All @@ -312,9 +306,7 @@ impl kvapi::TestSuite {
None,
Some(SeqV::with_meta(
3,
Some(KVMeta {
expire_at: Some(now + 10)
}),
Some(KVMeta::new_expire(now + 10)),
b"v2".to_vec()
))
]);
Expand All @@ -333,9 +325,7 @@ impl kvapi::TestSuite {
kv.upsert_kv(
UpsertKVReq::update("k2", b"v2")
.with(MatchSeq::Exact(3))
.with(KVMeta {
expire_at: Some(now - 1),
}),
.with(KVMeta::new_expire(now - 1)),
)
.await?;

Expand Down Expand Up @@ -368,9 +358,7 @@ impl kvapi::TestSuite {
test_key,
MatchSeq::Exact(seq + 1),
Operation::AsIs,
Some(KVMeta {
expire_at: Some(now + 20),
}),
Some(KVMeta::new_expire(now + 20)),
))
.await?;
assert_eq!(Some(SeqV::with_meta(1, None, b"v1".to_vec())), r.prev);
Expand All @@ -383,18 +371,14 @@ impl kvapi::TestSuite {
test_key,
MatchSeq::Exact(seq),
Operation::AsIs,
Some(KVMeta {
expire_at: Some(now + 20),
}),
Some(KVMeta::new_expire(now + 20)),
))
.await?;
assert_eq!(Some(SeqV::with_meta(1, None, b"v1".to_vec())), r.prev);
assert_eq!(
Some(SeqV::with_meta(
2,
Some(KVMeta {
expire_at: Some(now + 20)
}),
Some(KVMeta::new_expire(now + 20)),
b"v1".to_vec()
)),
r.result
Expand All @@ -404,13 +388,7 @@ impl kvapi::TestSuite {
let key_value = kv.get_kv(test_key).await?;
assert!(key_value.is_some());
assert_eq!(
SeqV::with_meta(
seq + 1,
Some(KVMeta {
expire_at: Some(now + 20)
}),
b"v1".to_vec()
),
SeqV::with_meta(seq + 1, Some(KVMeta::new_expire(now + 20)), b"v1".to_vec()),
key_value.unwrap(),
);

Expand Down
4 changes: 1 addition & 3 deletions src/meta/raft-store/src/applier.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,9 +381,7 @@ impl<'a> Applier<'a> {
put: &TxnPutRequest,
resp: &mut TxnReply,
) -> Result<(), io::Error> {
let upsert = UpsertKV::update(&put.key, &put.value).with(KVMeta {
expire_at: put.expire_at,
});
let upsert = UpsertKV::update(&put.key, &put.value).with(KVMeta::new(put.expire_at));

let (prev, _result) = self.upsert_kv(&upsert).await?;

Expand Down
68 changes: 19 additions & 49 deletions src/meta/raft-store/src/sm_v002/leveled_store/leveled_map_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,26 +379,18 @@ async fn build_2_level_with_meta() -> anyhow::Result<LeveledMap> {

// internal_seq: 0
l.str_map_mut()
.set(s("a"), Some((b("a0"), Some(KVMeta { expire_at: Some(1) }))))
.set(s("a"), Some((b("a0"), Some(KVMeta::new_expire(1)))))
.await?;
l.str_map_mut().set(s("b"), Some((b("b0"), None))).await?;
l.str_map_mut()
.set(s("c"), Some((b("c0"), Some(KVMeta { expire_at: Some(2) }))))
.set(s("c"), Some((b("c0"), Some(KVMeta::new_expire(2)))))
.await?;

l.freeze_writable();

// internal_seq: 3
l.str_map_mut()
.set(
s("b"),
Some((
b("b1"),
Some(KVMeta {
expire_at: Some(10),
}),
)),
)
.set(s("b"), Some((b("b1"), Some(KVMeta::new_expire(10)))))
.await?;
l.str_map_mut().set(s("c"), Some((b("c1"), None))).await?;

Expand All @@ -414,17 +406,17 @@ async fn test_two_level_update_value() -> anyhow::Result<()> {
let (prev, result) = MapApiExt::upsert_value(&mut l, s("a"), b("a1")).await?;
assert_eq!(
prev,
Marked::new_with_meta(1, b("a0"), Some(KVMeta { expire_at: Some(1) }))
Marked::new_with_meta(1, b("a0"), Some(KVMeta::new_expire(1)))
);
assert_eq!(
result,
Marked::new_with_meta(6, b("a1"), Some(KVMeta { expire_at: Some(1) }))
Marked::new_with_meta(6, b("a1"), Some(KVMeta::new_expire(1)))
);

let got = l.str_map().get("a").await?;
assert_eq!(
got,
Marked::new_with_meta(6, b("a1"), Some(KVMeta { expire_at: Some(1) }))
Marked::new_with_meta(6, b("a1"), Some(KVMeta::new_expire(1)))
);
}

Expand All @@ -435,23 +427,17 @@ async fn test_two_level_update_value() -> anyhow::Result<()> {
let (prev, result) = MapApiExt::upsert_value(&mut l, s("b"), b("x1")).await?;
assert_eq!(
prev,
Marked::new_normal(4, b("b1")).with_meta(Some(KVMeta {
expire_at: Some(10)
}))
Marked::new_normal(4, b("b1")).with_meta(Some(KVMeta::new_expire(10)))
);
assert_eq!(
result,
Marked::new_normal(6, b("x1")).with_meta(Some(KVMeta {
expire_at: Some(10)
}))
Marked::new_normal(6, b("x1")).with_meta(Some(KVMeta::new_expire(10)))
);

let got = l.str_map().get("b").await?;
assert_eq!(
got,
Marked::new_normal(6, b("x1")).with_meta(Some(KVMeta {
expire_at: Some(10)
}))
Marked::new_normal(6, b("x1")).with_meta(Some(KVMeta::new_expire(10)))
);
}

Expand All @@ -477,20 +463,20 @@ async fn test_two_level_update_meta() -> anyhow::Result<()> {
let mut l = build_2_level_with_meta().await?;

let (prev, result) =
MapApiExt::update_meta(&mut l, s("a"), Some(KVMeta { expire_at: Some(2) })).await?;
MapApiExt::update_meta(&mut l, s("a"), Some(KVMeta::new_expire(2))).await?;
assert_eq!(
prev,
Marked::new_with_meta(1, b("a0"), Some(KVMeta { expire_at: Some(1) }))
Marked::new_with_meta(1, b("a0"), Some(KVMeta::new_expire(1)))
);
assert_eq!(
result,
Marked::new_with_meta(6, b("a0"), Some(KVMeta { expire_at: Some(2) }))
Marked::new_with_meta(6, b("a0"), Some(KVMeta::new_expire(2)))
);

let got = l.str_map().get("a").await?;
assert_eq!(
got,
Marked::new_with_meta(6, b("a0"), Some(KVMeta { expire_at: Some(2) }))
Marked::new_with_meta(6, b("a0"), Some(KVMeta::new_expire(2)))
);
}

Expand All @@ -501,13 +487,7 @@ async fn test_two_level_update_meta() -> anyhow::Result<()> {
let (prev, result) = MapApiExt::update_meta(&mut l, s("b"), None).await?;
assert_eq!(
prev,
Marked::new_with_meta(
4,
b("b1"),
Some(KVMeta {
expire_at: Some(10)
})
)
Marked::new_with_meta(4, b("b1"), Some(KVMeta::new_expire(10)))
);
assert_eq!(result, Marked::new_with_meta(6, b("b1"), None));

Expand All @@ -519,28 +499,18 @@ async fn test_two_level_update_meta() -> anyhow::Result<()> {
{
let mut l = build_2_level_with_meta().await?;

let (prev, result) = MapApiExt::update_meta(
&mut l,
s("c"),
Some(KVMeta {
expire_at: Some(20),
}),
)
.await?;
let (prev, result) =
MapApiExt::update_meta(&mut l, s("c"), Some(KVMeta::new_expire(20))).await?;
assert_eq!(prev, Marked::new_with_meta(5, b("c1"), None));
assert_eq!(
result,
Marked::new_normal(6, b("c1")).with_meta(Some(KVMeta {
expire_at: Some(20)
}))
Marked::new_normal(6, b("c1")).with_meta(Some(KVMeta::new_expire(20)))
);

let got = l.str_map().get("c").await?;
assert_eq!(
got,
Marked::new_normal(6, b("c1")).with_meta(Some(KVMeta {
expire_at: Some(20)
}))
Marked::new_normal(6, b("c1")).with_meta(Some(KVMeta::new_expire(20)))
);
}

Expand All @@ -549,7 +519,7 @@ async fn test_two_level_update_meta() -> anyhow::Result<()> {
let mut l = build_2_level_with_meta().await?;

let (prev, result) =
MapApiExt::update_meta(&mut l, s("d"), Some(KVMeta { expire_at: Some(2) })).await?;
MapApiExt::update_meta(&mut l, s("d"), Some(KVMeta::new_expire(2))).await?;
assert_eq!(prev, Marked::new_tombstone(0));
assert_eq!(result, Marked::new_tombstone(0));

Expand Down
22 changes: 8 additions & 14 deletions src/meta/raft-store/src/sm_v002/marked/marked_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,9 @@ use crate::state_machine::ExpireValue;

#[test]
fn test_from_tuple() -> anyhow::Result<()> {
let m = Marked::from((1, 2u64, Some(KVMeta { expire_at: Some(3) })));
let m = Marked::from((1, 2u64, Some(KVMeta::new_expire(3))));

assert_eq!(
m,
Marked::new_with_meta(1, 2, Some(KVMeta { expire_at: Some(3) }))
);
assert_eq!(m, Marked::new_with_meta(1, 2, Some(KVMeta::new_expire(3))));

Ok(())
}
Expand All @@ -39,10 +36,10 @@ fn test_impl_trait_seq_value() -> anyhow::Result<()> {
assert_eq!(m.value(), Some(&2));
assert_eq!(m.meta(), None);

let m = Marked::new_with_meta(1, 2, Some(KVMeta { expire_at: Some(3) }));
let m = Marked::new_with_meta(1, 2, Some(KVMeta::new_expire(3)));
assert_eq!(m.seq(), 1);
assert_eq!(m.value(), Some(&2));
assert_eq!(m.meta(), Some(&KVMeta { expire_at: Some(3) }));
assert_eq!(m.meta(), Some(&KVMeta::new_expire(3)));

let m: Marked<u64> = Marked::new_tombstone(1);
assert_eq!(m.seq(), 0, "internal_seq is not returned to application");
Expand Down Expand Up @@ -79,11 +76,8 @@ fn test_unpack() -> anyhow::Result<()> {
let m = Marked::new_with_meta(1, 2, None);
assert_eq!(m.unpack_ref(), Some((&2, None)));

let m = Marked::new_with_meta(1, 2, Some(KVMeta { expire_at: Some(3) }));
assert_eq!(
m.unpack_ref(),
Some((&2, Some(&KVMeta { expire_at: Some(3) })))
);
let m = Marked::new_with_meta(1, 2, Some(KVMeta::new_expire(3)));
assert_eq!(m.unpack_ref(), Some((&2, Some(&KVMeta::new_expire(3)))));

let m: Marked<u64> = Marked::new_tombstone(1);
assert_eq!(m.unpack_ref(), None);
Expand Down Expand Up @@ -116,8 +110,8 @@ fn test_from_marked_for_option_seqv() -> anyhow::Result<()> {
let s: Option<SeqV<u64>> = Some(SeqV::new(1, 2));
assert_eq!(s, m.into());

let m = Marked::new_with_meta(1, 2, Some(KVMeta { expire_at: Some(3) }));
let s: Option<SeqV<u64>> = Some(SeqV::with_meta(1, Some(KVMeta { expire_at: Some(3) }), 2));
let m = Marked::new_with_meta(1, 2, Some(KVMeta::new_expire(3)));
let s: Option<SeqV<u64>> = Some(SeqV::with_meta(1, Some(KVMeta::new_expire(3)), 2));
assert_eq!(s, m.into());

let m: Marked<u64> = Marked::new_tombstone(1);
Expand Down
20 changes: 4 additions & 16 deletions src/meta/raft-store/src/sm_v002/marked/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,36 +256,24 @@ mod tests {
m
);

let m = m.with_meta(Some(KVMeta {
expire_at: Some(20),
}));
let m = m.with_meta(Some(KVMeta::new_expire(20)));

assert_eq!(
Marked::Normal {
internal_seq: 1,
value: "a",
meta: Some(KVMeta {
expire_at: Some(20)
})
meta: Some(KVMeta::new_expire(20))
},
m
);

let m = Marked::new_with_meta(
2,
"b",
Some(KVMeta {
expire_at: Some(30),
}),
);
let m = Marked::new_with_meta(2, "b", Some(KVMeta::new_expire(30)));

assert_eq!(
Marked::Normal {
internal_seq: 2,
value: "b",
meta: Some(KVMeta {
expire_at: Some(30)
})
meta: Some(KVMeta::new_expire(30))
},
m
);
Expand Down
Loading

0 comments on commit 0e0f62f

Please sign in to comment.