Skip to content

Commit

Permalink
maintain channel subscriptions in RAM (#9512)
Browse files Browse the repository at this point in the history
This avoids a giant database query on every leave/join event.

Release Notes:

- N/A
  • Loading branch information
ConradIrwin authored Mar 18, 2024
1 parent 963618a commit cd9b865
Show file tree
Hide file tree
Showing 7 changed files with 222 additions and 165 deletions.
14 changes: 5 additions & 9 deletions crates/collab/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ pub struct Channel {
}

impl Channel {
fn from_model(value: channel::Model) -> Self {
pub fn from_model(value: channel::Model) -> Self {
Channel {
id: value.id,
visibility: value.visibility,
Expand Down Expand Up @@ -604,16 +604,14 @@ pub struct RejoinedChannelBuffer {
#[derive(Clone)]
pub struct JoinRoom {
pub room: proto::Room,
pub channel_id: Option<ChannelId>,
pub channel_members: Vec<UserId>,
pub channel: Option<channel::Model>,
}

pub struct RejoinedRoom {
pub room: proto::Room,
pub rejoined_projects: Vec<RejoinedProject>,
pub reshared_projects: Vec<ResharedProject>,
pub channel_id: Option<ChannelId>,
pub channel_members: Vec<UserId>,
pub channel: Option<channel::Model>,
}

pub struct ResharedProject {
Expand Down Expand Up @@ -649,17 +647,15 @@ pub struct RejoinedWorktree {

pub struct LeftRoom {
pub room: proto::Room,
pub channel_id: Option<ChannelId>,
pub channel_members: Vec<UserId>,
pub channel: Option<channel::Model>,
pub left_projects: HashMap<ProjectId, LeftProject>,
pub canceled_calls_to_user_ids: Vec<UserId>,
pub deleted: bool,
}

pub struct RefreshedRoom {
pub room: proto::Room,
pub channel_id: Option<ChannelId>,
pub channel_members: Vec<UserId>,
pub channel: Option<channel::Model>,
pub stale_participant_user_ids: Vec<UserId>,
pub canceled_calls_to_user_ids: Vec<UserId>,
}
Expand Down
4 changes: 3 additions & 1 deletion crates/collab/src/db/ids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@ id_type!(NotificationKindId);
id_type!(HostedProjectId);

/// ChannelRole gives you permissions for both channels and calls.
#[derive(Eq, PartialEq, Copy, Clone, Debug, EnumIter, DeriveActiveEnum, Default, Hash)]
#[derive(
Eq, PartialEq, Copy, Clone, Debug, EnumIter, DeriveActiveEnum, Default, Hash, Serialize,
)]
#[sea_orm(rs_type = "String", db_type = "String(None)")]
pub enum ChannelRole {
/// Admin can read/write and change permissions.
Expand Down
53 changes: 10 additions & 43 deletions crates/collab/src/db/queries/channels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,7 @@ impl Database {
name: &str,
parent_channel_id: Option<ChannelId>,
admin_id: UserId,
) -> Result<(
Channel,
Option<channel_member::Model>,
Vec<channel_member::Model>,
)> {
) -> Result<(channel::Model, Option<channel_member::Model>)> {
let name = Self::sanitize_channel_name(name)?;
self.transaction(move |tx| async move {
let mut parent = None;
Expand Down Expand Up @@ -90,12 +86,7 @@ impl Database {
);
}

let channel_members = channel_member::Entity::find()
.filter(channel_member::Column::ChannelId.eq(channel.root_id()))
.all(&*tx)
.await?;

Ok((Channel::from_model(channel), membership, channel_members))
Ok((channel, membership))
})
.await
}
Expand Down Expand Up @@ -181,7 +172,7 @@ impl Database {
channel_id: ChannelId,
visibility: ChannelVisibility,
admin_id: UserId,
) -> Result<(Channel, Vec<channel_member::Model>)> {
) -> Result<channel::Model> {
self.transaction(move |tx| async move {
let channel = self.get_channel_internal(channel_id, &tx).await?;
self.check_user_is_channel_admin(&channel, admin_id, &tx)
Expand Down Expand Up @@ -214,12 +205,7 @@ impl Database {
model.visibility = ActiveValue::Set(visibility);
let channel = model.update(&*tx).await?;

let channel_members = channel_member::Entity::find()
.filter(channel_member::Column::ChannelId.eq(channel.root_id()))
.all(&*tx)
.await?;

Ok((Channel::from_model(channel), channel_members))
Ok(channel)
})
.await
}
Expand All @@ -245,21 +231,12 @@ impl Database {
&self,
channel_id: ChannelId,
user_id: UserId,
) -> Result<(Vec<ChannelId>, Vec<UserId>)> {
) -> Result<(ChannelId, Vec<ChannelId>)> {
self.transaction(move |tx| async move {
let channel = self.get_channel_internal(channel_id, &tx).await?;
self.check_user_is_channel_admin(&channel, user_id, &tx)
.await?;

let members_to_notify: Vec<UserId> = channel_member::Entity::find()
.filter(channel_member::Column::ChannelId.eq(channel.root_id()))
.select_only()
.column(channel_member::Column::UserId)
.distinct()
.into_values::<_, QueryUserIds>()
.all(&*tx)
.await?;

let channels_to_remove = self
.get_channel_descendants_excluding_self([&channel], &tx)
.await?
Expand All @@ -273,7 +250,7 @@ impl Database {
.exec(&*tx)
.await?;

Ok((channels_to_remove, members_to_notify))
Ok((channel.root_id(), channels_to_remove))
})
.await
}
Expand Down Expand Up @@ -343,7 +320,7 @@ impl Database {
channel_id: ChannelId,
admin_id: UserId,
new_name: &str,
) -> Result<(Channel, Vec<channel_member::Model>)> {
) -> Result<channel::Model> {
self.transaction(move |tx| async move {
let new_name = Self::sanitize_channel_name(new_name)?.to_string();

Expand All @@ -355,12 +332,7 @@ impl Database {
model.name = ActiveValue::Set(new_name.clone());
let channel = model.update(&*tx).await?;

let channel_members = channel_member::Entity::find()
.filter(channel_member::Column::ChannelId.eq(channel.root_id()))
.all(&*tx)
.await?;

Ok((Channel::from_model(channel), channel_members))
Ok(channel)
})
.await
}
Expand Down Expand Up @@ -984,7 +956,7 @@ impl Database {
channel_id: ChannelId,
new_parent_id: ChannelId,
admin_id: UserId,
) -> Result<(Vec<Channel>, Vec<channel_member::Model>)> {
) -> Result<(ChannelId, Vec<Channel>)> {
self.transaction(|tx| async move {
let channel = self.get_channel_internal(channel_id, &tx).await?;
self.check_user_is_channel_admin(&channel, admin_id, &tx)
Expand Down Expand Up @@ -1039,12 +1011,7 @@ impl Database {
.map(|c| Channel::from_model(c))
.collect::<Vec<_>>();

let channel_members = channel_member::Entity::find()
.filter(channel_member::Column::ChannelId.eq(root_id))
.all(&*tx)
.await?;

Ok((channels, channel_members))
Ok((root_id, channels))
})
.await
}
Expand Down
33 changes: 6 additions & 27 deletions crates/collab/src/db/queries/rooms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,7 @@ impl Database {
);

let (channel, room) = self.get_channel_room(room_id, &tx).await?;
let channel_members;
if let Some(channel) = &channel {
channel_members = self.get_channel_participants(channel, &tx).await?;
} else {
channel_members = Vec::new();

if channel.is_none() {
// Delete the room if it becomes empty.
if room.participants.is_empty() {
project::Entity::delete_many()
Expand All @@ -70,8 +65,7 @@ impl Database {

Ok(RefreshedRoom {
room,
channel_id: channel.map(|channel| channel.id),
channel_members,
channel,
stale_participant_user_ids,
canceled_calls_to_user_ids,
})
Expand Down Expand Up @@ -349,8 +343,7 @@ impl Database {
let room = self.get_room(room_id, &tx).await?;
Ok(JoinRoom {
room,
channel_id: None,
channel_members: vec![],
channel: None,
})
})
.await
Expand Down Expand Up @@ -446,11 +439,9 @@ impl Database {

let (channel, room) = self.get_channel_room(room_id, &tx).await?;
let channel = channel.ok_or_else(|| anyhow!("no channel for room"))?;
let channel_members = self.get_channel_participants(&channel, tx).await?;
Ok(JoinRoom {
room,
channel_id: Some(channel.id),
channel_members,
channel: Some(channel),
})
}

Expand Down Expand Up @@ -736,16 +727,10 @@ impl Database {
}

let (channel, room) = self.get_channel_room(room_id, &tx).await?;
let channel_members = if let Some(channel) = &channel {
self.get_channel_participants(&channel, &tx).await?
} else {
Vec::new()
};

Ok(RejoinedRoom {
room,
channel_id: channel.map(|channel| channel.id),
channel_members,
channel,
rejoined_projects,
reshared_projects,
})
Expand Down Expand Up @@ -902,15 +887,9 @@ impl Database {
false
};

let channel_members = if let Some(channel) = &channel {
self.get_channel_participants(channel, &tx).await?
} else {
Vec::new()
};
let left_room = LeftRoom {
room,
channel_id: channel.map(|channel| channel.id),
channel_members,
channel,
left_projects,
canceled_calls_to_user_ids,
deleted,
Expand Down
3 changes: 1 addition & 2 deletions crates/collab/src/db/tests/channel_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,9 @@ async fn test_channels(db: &Arc<Database>) {
assert!(db.get_channel(crdb_id, a_id).await.is_err());

// Remove a channel tree
let (mut channel_ids, user_ids) = db.delete_channel(rust_id, a_id).await.unwrap();
let (_, mut channel_ids) = db.delete_channel(rust_id, a_id).await.unwrap();
channel_ids.sort();
assert_eq!(channel_ids, &[rust_id, cargo_id, cargo_ra_id]);
assert_eq!(user_ids, &[a_id]);

assert!(db.get_channel(rust_id, a_id).await.is_err());
assert!(db.get_channel(cargo_id, a_id).await.is_err());
Expand Down
Loading

0 comments on commit cd9b865

Please sign in to comment.