RocksDB stress test fixes + find_merge_thread() bugfix

This commit is contained in:
mdecimus
2023-12-20 17:06:32 +01:00
parent f7313eecaf
commit d4aca0a8e0
28 changed files with 819 additions and 268 deletions

View File

@@ -56,7 +56,7 @@ use utils::map::vec_map::VecMap;
use crate::{auth::AccessToken, mailbox::UidMailbox, services::housekeeper::Event, Bincode, JMAP};
use super::{
index::{EmailIndexBuilder, TrimTextValue, MAX_SORT_FIELD_LENGTH},
index::{EmailIndexBuilder, TrimTextValue, VisitValues, MAX_ID_LENGTH, MAX_SORT_FIELD_LENGTH},
ingest::IngestedEmail,
metadata::MessageMetadata,
};
@@ -322,30 +322,34 @@ impl JMAP {
}
// Obtain threadId
let mut references = vec![];
let mut references = Vec::with_capacity(5);
let mut subject = "";
for header in &metadata.contents.parts[0].headers {
match header.name {
match &header.name {
HeaderName::MessageId
| HeaderName::InReplyTo
| HeaderName::References
| HeaderName::ResentMessageId => match &header.value {
HeaderValue::Text(text) => {
references.push(text.as_ref());
}
HeaderValue::TextList(list) => {
references.extend(list.iter().map(|v| v.as_ref()));
}
_ => (),
},
HeaderName::Subject => {
if let HeaderValue::Text(value) = &header.value {
subject = thread_name(value).trim_text(MAX_SORT_FIELD_LENGTH);
}
| HeaderName::ResentMessageId => {
header.value.visit_text(|id| {
if !id.is_empty() && id.len() < MAX_ID_LENGTH {
references.push(id);
}
});
}
HeaderName::Subject if subject.is_empty() => {
subject = thread_name(match &header.value {
HeaderValue::Text(text) => text.as_ref(),
HeaderValue::TextList(list) if !list.is_empty() => {
list.first().unwrap().as_ref()
}
_ => "",
})
.trim_text(MAX_SORT_FIELD_LENGTH);
}
_ => (),
}
}
let thread_id = if !references.is_empty() {
self.find_or_merge_thread(account_id, subject, &references)
.await

View File

@@ -140,7 +140,7 @@ impl JMAP {
mailbox_ids,
keywords: email.keywords,
received_at: email.received_at.map(|r| r.into()),
skip_duplicates: false,
skip_duplicates: true,
encrypt: self.config.encrypt && self.config.encrypt_append,
})
.await

View File

@@ -184,14 +184,20 @@ impl IndexMessage for BatchBuilder {
}
match header.name {
HeaderName::MessageId
| HeaderName::InReplyTo
| HeaderName::References
| HeaderName::ResentMessageId => {
HeaderName::MessageId => {
header.value.visit_text(|id| {
// Add ids to inverted index
if id.len() < MAX_ID_LENGTH {
self.value(Property::MessageId, id, F_INDEX | options);
self.value(Property::References, id, F_INDEX | options);
}
});
}
HeaderName::InReplyTo | HeaderName::References | HeaderName::ResentMessageId => {
header.value.visit_text(|id| {
// Add ids to inverted index
if id.len() < MAX_ID_LENGTH {
self.value(Property::References, id, F_INDEX | options);
}
});
}
@@ -523,21 +529,21 @@ impl GetContentLanguage for MessagePart<'_> {
}
}
trait VisitValues {
fn visit_addresses(&self, visitor: impl FnMut(AddressElement, &str));
fn visit_text(&self, visitor: impl FnMut(&str));
pub trait VisitValues<'x> {
fn visit_addresses<'y: 'x>(&'y self, visitor: impl FnMut(AddressElement, &'x str));
fn visit_text<'y: 'x>(&'y self, visitor: impl FnMut(&'x str));
fn into_visit_text(self, visitor: impl FnMut(String));
}
#[derive(Debug, PartialEq, Eq)]
enum AddressElement {
pub enum AddressElement {
Name,
Address,
GroupName,
}
impl VisitValues for HeaderValue<'_> {
fn visit_addresses(&self, mut visitor: impl FnMut(AddressElement, &str)) {
impl<'x> VisitValues<'x> for HeaderValue<'x> {
fn visit_addresses<'y: 'x>(&'y self, mut visitor: impl FnMut(AddressElement, &'x str)) {
match self {
HeaderValue::Address(Address::List(addr_list)) => {
for addr in addr_list {
@@ -569,7 +575,7 @@ impl VisitValues for HeaderValue<'_> {
}
}
fn visit_text(&self, mut visitor: impl FnMut(&str)) {
fn visit_text<'y: 'x>(&'y self, mut visitor: impl FnMut(&'x str)) {
match &self {
HeaderValue::Text(text) => {
visitor(text.as_ref());

View File

@@ -21,7 +21,7 @@
* for more details.
*/
use std::borrow::Cow;
use std::{borrow::Cow, time::Duration};
use jmap_proto::{
object::Object,
@@ -34,6 +34,7 @@ use mail_parser::{
parsers::fields::thread::thread_name, HeaderName, HeaderValue, Message, PartType,
};
use rand::Rng;
use store::{
ahash::AHashSet,
query::Filter,
@@ -46,7 +47,7 @@ use store::{
use utils::map::vec_map::VecMap;
use crate::{
email::index::{IndexMessage, MAX_ID_LENGTH},
email::index::{IndexMessage, VisitValues, MAX_ID_LENGTH},
mailbox::UidMailbox,
services::housekeeper::Event,
IngestError, JMAP,
@@ -77,6 +78,8 @@ pub struct IngestEmail<'x> {
pub encrypt: bool,
}
const MAX_RETRIES: u32 = 10;
impl JMAP {
#[allow(clippy::blocks_in_if_conditions)]
pub async fn email_ingest(
@@ -107,24 +110,26 @@ impl JMAP {
let thread_id = {
let mut references = Vec::with_capacity(5);
let mut subject = "";
let mut message_id = "";
for header in message.root_part().headers().iter().rev() {
match header.name {
HeaderName::MessageId
| HeaderName::InReplyTo
| HeaderName::References
| HeaderName::ResentMessageId => match &header.value {
HeaderValue::Text(id) if id.len() < MAX_ID_LENGTH => {
references.push(id.as_ref());
}
HeaderValue::TextList(ids) => {
for id in ids {
if id.len() < MAX_ID_LENGTH {
references.push(id.as_ref());
}
match &header.name {
HeaderName::MessageId => header.value.visit_text(|id| {
if !id.is_empty() && id.len() < MAX_ID_LENGTH {
if message_id.is_empty() {
message_id = id;
}
references.push(id);
}
_ => (),
},
}),
HeaderName::InReplyTo
| HeaderName::References
| HeaderName::ResentMessageId => {
header.value.visit_text(|id| {
if !id.is_empty() && id.len() < MAX_ID_LENGTH {
references.push(id);
}
});
}
HeaderName::Subject if subject.is_empty() => {
subject = thread_name(match &header.value {
HeaderValue::Text(text) => text.as_ref(),
@@ -141,24 +146,21 @@ impl JMAP {
// Check for duplicates
if params.skip_duplicates
&& !references.is_empty()
&& !message_id.is_empty()
&& !self
.store
.filter(
params.account_id,
Collection::Email,
references
.iter()
.map(|id| Filter::eq(Property::MessageId, *id))
.collect(),
vec![Filter::eq(Property::MessageId, message_id)],
)
.await
.map_err(|err| {
tracing::error!(
event = "error",
context = "find_duplicates",
error = ?err,
"Duplicate message search failed.");
event = "error",
context = "find_duplicates",
error = ?err,
"Duplicate message search failed.");
IngestError::Temporary
})?
.results
@@ -383,7 +385,7 @@ impl JMAP {
));
filters.push(Filter::Or);
for reference in references {
filters.push(Filter::eq(Property::MessageId, *reference));
filters.push(Filter::eq(Property::References, *reference));
}
filters.push(Filter::End);
let results = self
@@ -515,7 +517,9 @@ impl JMAP {
match self.store.write(batch.build()).await {
Ok(_) => return Ok(Some(thread_id)),
Err(store::Error::AssertValueFailed) if try_count < 3 => {
Err(store::Error::AssertValueFailed) if try_count < MAX_RETRIES => {
let backoff = rand::thread_rng().gen_range(50..=300);
tokio::time::sleep(Duration::from_millis(backoff)).await;
try_count += 1;
}
Err(err) => {

View File

@@ -171,6 +171,7 @@ pub struct Bincode<T: serde::Serialize + serde::de::DeserializeOwned> {
pub inner: T,
}
#[derive(Debug)]
pub enum IngestError {
Temporary,
OverQuota,

View File

@@ -241,6 +241,7 @@ impl JMAP {
stack.push((children, it));
}
}
debug_assert_eq!(response.ids.len(), paginate.ids.len(), "{tree:#?}");
response.update_results(paginate.build())?;
} else {
response = self

View File

@@ -46,7 +46,11 @@ use jmap_proto::{
use store::{
query::Filter,
roaring::RoaringBitmap,
write::{assert::HashedValue, log::ChangeLogBuilder, BatchBuilder, F_BITMAP, F_CLEAR, F_VALUE},
write::{
assert::{AssertValue, HashedValue},
log::ChangeLogBuilder,
BatchBuilder, F_BITMAP, F_CLEAR, F_VALUE,
},
};
use crate::{
@@ -115,13 +119,45 @@ impl JMAP {
.await?;
batch
.with_account_id(account_id)
.with_collection(Collection::Mailbox)
.create_document(document_id)
.custom(builder);
.with_collection(Collection::Mailbox);
if let Value::Id(parent_id) =
builder.changes().unwrap().get(&Property::ParentId)
{
let parent_id = parent_id.document_id();
if parent_id > 0 {
batch
.update_document(parent_id - 1)
.assert_value(Property::Value, AssertValue::Some);
}
}
batch.create_document(document_id).custom(builder);
changes.log_insert(Collection::Mailbox, document_id);
ctx.mailbox_ids.insert(document_id);
self.write_batch(batch).await?;
ctx.response.created(id, document_id);
match self.store.write(batch.build()).await {
Ok(_) => {
ctx.response.created(id, document_id);
}
Err(store::Error::AssertValueFailed) => {
ctx.response.not_created.append(
id,
SetError::forbidden().with_description(
"Another process deleted the parent mailbox, please try again.",
),
);
continue 'create;
}
Err(err) => {
tracing::error!(
event = "error",
context = "mailbox_set",
account_id = account_id,
error = ?err,
"Failed to update mailbox(es).");
return Err(MethodError::ServerPartialFail);
}
}
}
Err(err) => {
ctx.response.not_created.append(id, err);
@@ -182,9 +218,21 @@ impl JMAP {
let mut batch = BatchBuilder::new();
batch
.with_account_id(account_id)
.with_collection(Collection::Mailbox)
.update_document(document_id)
.custom(builder);
.with_collection(Collection::Mailbox);
if let Value::Id(parent_id) =
builder.changes().unwrap().get(&Property::ParentId)
{
let parent_id = parent_id.document_id();
if parent_id > 0 {
batch
.update_document(parent_id - 1)
.assert_value(Property::Value, AssertValue::Some);
}
}
batch.update_document(document_id).custom(builder);
if !batch.is_empty() {
match self.store.write(batch.build()).await {
Ok(_) => {