feat: Remove hidden relays automatically by j-g00da · Pull Request #8402 · chatmail/core

j-g00da · 2026-07-07T06:28:49Z

Automatically remove relays that are hidden (is_published=0) and haven't been used to receive new messages for over 90 days.

Closes: #8384

j-g00da · 2026-07-07T06:43:22Z

There's one open question: #8384 (comment)

CI fails due to: #8403

Hocuri

Looks good, only some minor things.

Automatically remove relays that are hidden (`is_published=0`) and haven't been used to receive new messages for over 90 days. Closes: #8384 Signed-off-by: Jagoda Ślązak <jslazak@jslazak.com>

iequidoo · 2026-07-13T12:58:18Z

    /// so that we don't cause extra messages to the corresponding inbox,
    /// but can still receive messages from contacts who don't know our new transport addresses yet.
+    ///
+    /// Unpublished transports that are not used to receive any new messages for 90 days


Suggested change

/// Unpublished transports that are not used to receive any new messages for 90 days

/// Unpublished transports that are not used to receive any new messages for [`crate::sql::HIDDEN_TRANSPORT_CLEANUP_TIME`]

iequidoo · 2026-07-13T12:58:24Z

        .context("add_parts error")?
    };

+    // NOTE: this is called only for first transport that receives a message.


Suggested change

// NOTE: this is called only for first transport that receives a message.

// NOTE: normally this only updates the first transport that receives a message.

It's not actually "called" for the first transport, and it can update multiple transports if receive_imf failed previously for the same message received from another transport.

Btw, why not do this from imap right before receive_imf_inner() where we also know the transport id? Currently if an error happens before this place, we don't update transport's timestamp, but it doesn't mean that we don't use the transport to receive the message, moreover, the message may have useful side effects.

iequidoo · 2026-07-13T12:58:28Z

+    // it's safe to assume that the hidden transport can be safely removed.
+    sql::update_transport_last_rcvd_timestamp(context, rfc724_mid)
+        .await
+        .context("Failed to update transport's last_rcvd_timestamp value.")?;


Suggested change

.context("Failed to update transport's last_rcvd_timestamp value.")?;

.context("Failed to update transport's last_rcvd_timestamp value")?;

The reason for not adding a point here is that if you add a point here, and then log the error with warn!(context, "Something failed: {err:#}"), then there will be two points at the end of the log line. See also https://github.com/chatmail/core/blob/main/STYLE.md#errors

iequidoo · 2026-07-13T12:58:32Z

 use pool::{Pool, WalCheckpointStats};

+/// How long a hidden and unused transport should be kept in the database before being deleted.
+pub const HIDDEN_TRANSPORT_CLEANUP_TIME: i64 = 90 * 24 * 60 * 60;


Could be renamed to UNPUBLISHED_TRANSPORT_CLEANUP_TIME for consistency with set_transport_unpublished(). Maybe even to UNPUBLISHED_TRANSPORT_KEEP_TIME or UNPUBLISHED_TRANSPORT_TTL

iequidoo · 2026-07-13T12:58:34Z


+/// Removes transports that are hidden (`is_published=0`),
+/// and haven't been used to receive new messages for [`HIDDEN_TRANSPORT_CLEANUP_TIME`] seconds.
+pub async fn remove_unused_hidden_transports(context: &Context) -> Result<usize> {


Suggested change

pub async fn remove_unused_hidden_transports(context: &Context) -> Result<usize> {

pub(crate) async fn remove_unused_hidden_transports(context: &Context) -> Result<usize> {

iequidoo · 2026-07-13T12:58:37Z

+///
+/// This is used to postpone deletion of hidden transport by [`remove_unused_hidden_transports`],
+/// if it is still used to receive messages.
+pub async fn update_transport_last_rcvd_timestamp(


Suggested change

pub async fn update_transport_last_rcvd_timestamp(

pub(crate) async fn update_transport_last_rcvd_timestamp(

Hocuri · 2026-07-13T13:19:27Z

+            "UPDATE transports
+            SET last_rcvd_timestamp=?1
+            FROM transports t1
+            INNER JOIN imap ON t1.id=imap.transport_id
+            WHERE imap.rfc724_mid=?2",


This would seem more readable to me as

UPDATE transports SET last_rcvd_timestamp = ?1 WHERE id = ( SELECT transport_id FROM imap WHERE imap.rfc724_mid = ?2 )

because it wouldn't be necessary to know the UPDATE FROM SQL syntax, and it would be possible to think about the inner and the outer statement separately.

j-g00da requested review from Hocuri and link2xt and removed request for link2xt July 7, 2026 06:43

j-g00da force-pushed the j-g00da/remove-hidden-relays-automatically branch from 921c5df to 1b3f434 Compare July 7, 2026 15:15

link2xt reviewed Jul 7, 2026

View reviewed changes

Comment thread src/sql.rs Outdated

Hocuri mentioned this pull request Jul 8, 2026

api!: delete_transport() must not be used by UIs anymore. Instead, set_transport_unpublished() must be called when a user clicks on "Remove" #8408

Merged

j-g00da force-pushed the j-g00da/remove-hidden-relays-automatically branch from 1b3f434 to a0c7c59 Compare July 8, 2026 13:46

Hocuri reviewed Jul 8, 2026

View reviewed changes

Comment thread src/sql/sql_tests.rs Outdated

Comment thread src/sql/sql_tests.rs Outdated

Comment thread src/sql/sql_tests.rs

Comment thread src/sql/sql_tests.rs Outdated

Comment thread src/sql.rs Outdated

j-g00da force-pushed the j-g00da/remove-hidden-relays-automatically branch 4 times, most recently from 3607f29 to 967d87f Compare July 10, 2026 12:02

j-g00da requested review from Hocuri and link2xt July 10, 2026 12:10

Hocuri reviewed Jul 10, 2026

View reviewed changes

Comment thread src/receive_imf.rs

Hocuri reviewed Jul 10, 2026

View reviewed changes

Comment thread src/sql.rs Outdated

feat: Remove hidden relays automatically

c18ac54

Automatically remove relays that are hidden (`is_published=0`) and haven't been used to receive new messages for over 90 days. Closes: #8384 Signed-off-by: Jagoda Ślązak <jslazak@jslazak.com>

j-g00da force-pushed the j-g00da/remove-hidden-relays-automatically branch from 967d87f to c18ac54 Compare July 13, 2026 09:31

j-g00da requested review from Hocuri and iequidoo July 13, 2026 09:34

iequidoo reviewed Jul 13, 2026

View reviewed changes

Hocuri approved these changes Jul 13, 2026

View reviewed changes

	/// Unpublished transports that are not used to receive any new messages for 90 days
	/// Unpublished transports that are not used to receive any new messages for [`crate::sql::HIDDEN_TRANSPORT_CLEANUP_TIME`]

	// NOTE: this is called only for first transport that receives a message.
	// NOTE: normally this only updates the first transport that receives a message.

	.context("Failed to update transport's last_rcvd_timestamp value.")?;
	.context("Failed to update transport's last_rcvd_timestamp value")?;

	pub async fn remove_unused_hidden_transports(context: &Context) -> Result<usize> {
	pub(crate) async fn remove_unused_hidden_transports(context: &Context) -> Result<usize> {

	pub async fn update_transport_last_rcvd_timestamp(
	pub(crate) async fn update_transport_last_rcvd_timestamp(

Uh oh!

Uh oh!

Conversation

j-g00da commented Jul 7, 2026

Uh oh!

j-g00da commented Jul 7, 2026

Uh oh!

Uh oh!

Hocuri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants