In-memory DatabaseLogger #3961
In-memory DatabaseLogger #3961
Conversation
kim
force-pushed
the
kim/in-memory-module-logs
branch
from
3e71368 to
fe02d88
Compare
kim
force-pushed
the
kim/in-memory-module-logs
branch
2 times, most recently
from
fa8fb90 to
5c76b8e
Compare
kim
force-pushed
the
kim/in-memory-module-logs
branch
from
5c76b8e to
9ce1579
Compare
| .map(Ok::<_, std::convert::Infallible>); | ||
| let Some(host) = worker_ctx.leader(database.id).await.map_err(log_and_500)? else { | ||
| if follow { | ||
| return Err((StatusCode::NOT_FOUND, "Host not running, cannot follow logs").into()); |
There was a problem hiding this comment.
I think this is a misleading error message. My reading of the doc comment on NodeDelegate::leader is that it will spawn the host if necessary, and will only return None for databases whose leader is not assigned to this node. (I am unsure of the interaction with in-memory databases, if that's relevant in this path.)
There was a problem hiding this comment.
Hm you're right. This has been incorrect before because it isn't considered that this might be the right host to read disk-backed module logs from, but the database is not running (e.g. because it is suspended, or currently bootstrapping).
For now, I'll revert to return 404 with the message of the get_leader_replica_by_database call above, and remove that call as it is redundant.
I'll come up with a follow-up.
kim
force-pushed
the
kim/in-memory-module-logs
branch
from
2be2fd8 to
1486274
Compare
kim
force-pushed
the
kim/in-memory-module-logs
branch
from
1486274 to
87525f8
Compare
crates/core/src/database_logger.rs
Outdated
| // Cap reading the tail into memory at a few hundred KiB. | ||
| let n = n.min(2500); | ||
| let (tail, more) = { | ||
| let inner = self.inner.lock_arc(); |
There was a problem hiding this comment.
This seems questionable to me - won't this block if another thread is in the process of doing the sync_data() call below?
There was a problem hiding this comment.
If we want to do this we should just make it an async mutex.
There was a problem hiding this comment.
We have to do this if we don't want to serve duplicate log lines.
Using an async mutex doesn't work, though, as we are calling DatabaseLogger:write from within an async context virtually everywhere. I guess we always had a problem with blocking here.
There was a problem hiding this comment.
Made it to use message-passing. PTAL
kim
force-pushed
the
kim/in-memory-module-logs
branch
3 times, most recently
from
a16cf24 to
c7ad80b
Compare
kim
force-pushed
the
kim/in-memory-module-logs
branch
2 times, most recently
from
edb5dc8 to
38524a5
Compare
This is the second step to make in-memory-only databases not touch the disk at all. While at it, also make it so file-backed module logs are streamed in constant memory where possible. Co-authored-by: Phoebe Goldman <[email protected]> Signed-off-by: Kim Altintop <[email protected]>
kim
force-pushed
the
kim/in-memory-module-logs
branch
from
38524a5 to
84ff62d
Compare
This is the second step to make in-memory-only databases not touch the disk at all.
While at it, also make it so file-backed module logs are streamed in constant memory where possible.
Depends-on: #3912
Expected complexity level and risk
2
Testing
Added some unit-level tests.