Skip to content

logseq#12539: reduces memory allocation for graph file query to avoid crash; adds debug output #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vr-one wants to merge 1 commit into
base: master
Choose a base branch
from
Open

logseq#12539: reduces memory allocation for graph file query to avoid crash; adds debug output #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/rsapi/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ pub async fn get_local_all_files_meta(
let start_time = Instant::now();

let graph = implementation::get_graph(&graph_uuid)?;
log::trace!("get local all files meta: base_path={:?}", base_path);
let files_meta: HashMap<String, FileMeta> = graph
.get_all_files_meta(&base_path)
.await?
Expand Down Expand Up @@ -377,5 +378,7 @@ pub fn decrypt_fnames(graph_uuid: String, fnames: Vec<String>) -> Result<Vec<Str
pub async fn canonicalize_path(file_path: String) -> Result<String> {
let new_path = std::fs::canonicalize(PathBuf::from(file_path))?;
let strip_windows_prefix = dunce::canonicalize(new_path)?;
Ok(strip_windows_prefix.to_str().unwrap().to_string())
Ok(strip_windows_prefix.to_str()
.unwrap_or_else(|| panic!("rsapi: canonicalize_path: non-UTF8 path {:?}", strip_windows_prefix))
.to_string())
}
15 changes: 10 additions & 5 deletions rsapi-impl/src/graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,7 @@ impl Graph {
}
};
tasks.push(async move {
log::trace!("rsapi: update_remote_file: {}", file_path);
let content = fs::read(full_file_path).await?;
// stage 1.1: md5 metadata
let md5checksum = md5_hexdigest(&content);
Expand Down Expand Up @@ -601,6 +602,7 @@ impl Graph {
base_path: P,
file_path: S,
) -> Result<FileMeta> {
log::trace!("rsapi: get_file_meta: {}", file_path.as_ref());
use md5::{Digest, Md5};
let base_path = dunce::canonicalize(base_path.as_ref())?;
let full_file_path = dunce::canonicalize(base_path.join(file_path.as_ref()))?;
Expand All @@ -616,7 +618,9 @@ impl Graph {
let mut file = fs::File::open(&full_file_path).await?;

let mut nread = 0;
let mut buf = Vec::with_capacity(1024 * 1024);
// 64KB chunks — was 1MB, which caused OOM when join_all runs hundreds of
// get_file_meta futures concurrently (each holding a 1MB buffer alive).
let mut buf = Vec::with_capacity(64 * 1024);
let mut hasher = Md5::new();
loop {
let n = file.read_buf(&mut buf).await?;
Expand Down Expand Up @@ -692,12 +696,13 @@ fn is_page_file(file_path: &str) -> bool {
}

fn is_page_file_path<P: AsRef<Path>>(file_path: P) -> bool {
let t = file_path
.as_ref()
let path = file_path.as_ref();
let fname = path
.file_name()
.unwrap()
.unwrap_or_else(|| panic!("rsapi: is_page_file_path: no filename component in path {:?}", path));
let t = fname
.to_str()
.unwrap()
.unwrap_or_else(|| panic!("rsapi: is_page_file_path: non-UTF8 filename in path {:?}", path))
.to_lowercase();
t.ends_with(".md") || t.ends_with(".org") || t.ends_with(".markdown")
}
14 changes: 9 additions & 5 deletions sync/src/sync.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,14 @@ impl SyncClient {
.http2_keep_alive_timeout(Duration::from_secs(60))
.http2_keep_alive_while_idle(true);
if let Some(proxy) = unsafe { HTTPS_PROXY.as_ref() } {
builder = builder.proxy(reqwest::Proxy::https(proxy).unwrap());
builder = builder.proxy(reqwest::Proxy::https(proxy)
.expect("rsapi: invalid HTTPS_PROXY URL"));
}
if accept_invalid_certs {
// log::info!("NODE_TLS_REJECT_UNAUTHORIZED=0, won't validate certs");
builder = builder.danger_accept_invalid_certs(true);
}
builder.build().unwrap()
builder.build().expect("rsapi: failed to build reqwest HTTP client")
};

SyncClient {
Expand Down Expand Up @@ -327,7 +328,7 @@ impl SyncClient {
credential.s3_prefix = credential
.s3_prefix
.strip_prefix(bucket())
.unwrap()
.unwrap_or_else(|| panic!("rsapi: s3_prefix {:?} does not start with bucket {:?}", credential.s3_prefix, bucket()))
.trim_start_matches('/')
.to_owned()
+ "/";
Expand Down Expand Up @@ -393,7 +394,8 @@ impl SyncClient {
use s3_presign::Bucket;
use s3_presign::Credentials;

let credentials = self.credentials.as_ref().unwrap();
let credentials = self.credentials.as_ref()
.expect("rsapi: upload_tempfile called before refresh_temp_credential");

let credentials = Credentials::new(
credentials.access_key_id.clone(),
Expand All @@ -402,7 +404,9 @@ impl SyncClient {
);
let bucket = Bucket::new(region(), bucket());

let key = self.s3_prefix.clone().unwrap() + &*random_string(12);
let key = self.s3_prefix.clone()
.expect("rsapi: s3_prefix not set — refresh_temp_credential must be called first")
+ &*random_string(12);
// 1 hour expiration
let presign_url = s3_presign::put(&credentials, &bucket, &key, 60 * 60)
.ok_or(SyncError::Custom("can not generate presign url".to_owned()))?;
Expand Down