Skip to content

Commit fb116e1

Browse files
committed
this does fix #6580 by adding polling every 7.5 seconds to check readonly status
- chokidar with polling doesn't even report change in permissions, hence the original cause of #6580 - cleans up and refactors various code to prevent bugs and confusion, but shouldn't actually change anything - I noticed issues with watching + polling while testing this and will address that in another commit
1 parent ea33cd2 commit fb116e1

File tree

7 files changed

+109
-60
lines changed

7 files changed

+109
-60
lines changed

src/compute/compute/lib/listings.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ export async function initListings({
5252
project_id,
5353
compute_server_id,
5454
getListing,
55-
createWatcher: (path: string, debounceMs: number) =>
56-
new Watcher(path, debounceMs),
55+
createWatcher: (path: string, debounce: number) =>
56+
new Watcher(path, { debounce }),
5757
onDeletePath: (path) => {
5858
logger.debug("onDeletePath -- TODO:", { path });
5959
},

src/packages/backend/path-watcher.ts

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
*/
55

66
/*
7+
Watch A DIRECTORY for changes. Use ./watcher.ts for a single file.
8+
9+
710
Slightly generalized fs.watch that works even when the directory doesn't exist,
811
but also doesn't provide any information about what changed.
912
@@ -76,11 +79,18 @@ export class Watcher extends EventEmitter {
7679
private exists: boolean;
7780
private watchContents?: FSWatcher;
7881
private watchExistence?: FSWatcher;
82+
private interval_ms: number;
7983
private debounce_ms: number;
8084
private debouncedChange: any;
8185
private log: Function;
8286

83-
constructor(path: string, debounce_ms: number) {
87+
constructor(
88+
path: string,
89+
{
90+
debounce: debounce_ms = 0,
91+
interval: interval_ms,
92+
}: { debounce?: number; interval?: number } = {},
93+
) {
8494
super();
8595
this.log = logger.extend(path).debug;
8696
this.log(`initializing: poll=${POLLING}`);
@@ -89,10 +99,13 @@ export class Watcher extends EventEmitter {
8999
}
90100
this.path = path.startsWith("/") ? path : join(process.env.HOME, path);
91101
this.debounce_ms = debounce_ms;
92-
this.debouncedChange = debounce(this.change.bind(this), this.debounce_ms, {
93-
leading: true,
94-
trailing: true,
95-
}).bind(this);
102+
this.interval_ms = interval_ms ?? DEFAULT_POLL_MS;
103+
this.debouncedChange = this.debounce_ms
104+
? debounce(this.change, this.debounce_ms, {
105+
leading: true,
106+
trailing: true,
107+
}).bind(this)
108+
: this.change;
96109
this.init();
97110
}
98111

@@ -109,8 +122,16 @@ export class Watcher extends EventEmitter {
109122
}
110123
}
111124

125+
private chokidarOptions = () => {
126+
return {
127+
...ChokidarOpts,
128+
interval: this.interval_ms,
129+
binaryInterval: this.interval_ms,
130+
};
131+
};
132+
112133
private initWatchContents(): void {
113-
this.watchContents = watch(this.path, ChokidarOpts);
134+
this.watchContents = watch(this.path, this.chokidarOptions());
114135
this.watchContents.on("all", this.debouncedChange);
115136
this.watchContents.on("error", (err) => {
116137
this.log(`error watching listings -- ${err}`);
@@ -119,7 +140,7 @@ export class Watcher extends EventEmitter {
119140

120141
private async initWatchExistence(): Promise<void> {
121142
const containing_path = path_split(this.path).head;
122-
this.watchExistence = watch(containing_path, ChokidarOpts);
143+
this.watchExistence = watch(containing_path, this.chokidarOptions());
123144
this.watchExistence.on("all", this.watchExistenceChange(containing_path));
124145
this.watchExistence.on("error", (err) => {
125146
this.log(`error watching for existence of ${this.path} -- ${err}`);
@@ -147,9 +168,9 @@ export class Watcher extends EventEmitter {
147168
}
148169
};
149170

150-
private change(): void {
171+
private change = (): void => {
151172
this.emit("change");
152-
}
173+
};
153174

154175
public close(): void {
155176
this.watchExistence?.close();

src/packages/backend/watcher.ts

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@
44
*/
55

66
/*
7-
Watch a file for changes
7+
Watch one SINGLE FILE for changes. Use ./path-watcher.ts for a directory.
88
99
Watch for changes to the given file. Returns obj, which
1010
is an event emitter with events:
1111
12-
- 'change', ctime - when file changes or is created
12+
- 'change', ctime, stats - when file changes or is created
1313
- 'delete' - when file is deleted
1414
1515
and a method .close().
@@ -25,30 +25,30 @@ import { watch, FSWatcher } from "chokidar";
2525
import { getLogger } from "./logger";
2626
import { debounce as lodashDebounce } from "lodash";
2727

28-
const L = getLogger("watcher");
28+
const logger = getLogger("backend:watcher");
2929

3030
export class Watcher extends EventEmitter {
3131
private path: string;
32-
private interval: number;
3332
private watcher: FSWatcher;
3433

35-
constructor(path: string, interval: number = 300, debounce: number = 0) {
34+
constructor(
35+
path: string,
36+
{ debounce, interval = 300 }: { debounce?: number; interval?: number } = {},
37+
) {
3638
super();
3739
this.path = path;
38-
this.interval = interval;
3940

40-
L.debug(`${path}: interval=${interval}, debounce=${debounce}`);
41+
logger.debug({ path, debounce, interval });
4142
this.watcher = watch(this.path, {
42-
interval: this.interval,
43+
interval,
4344
// polling is critical for network mounted file systems,
4445
// and given architecture of cocalc there is no easy way around this.
4546
// E.g., on compute servers, everything breaks involving sync or cloudfs,
4647
// and in shared project s3/gcsfuse/sshfs would all break. So we
4748
// use polling.
4849
usePolling: true,
49-
persistent: false,
50+
persistent: true,
5051
alwaysStat: true,
51-
atomic: true,
5252
});
5353
this.watcher.on("unlink", () => {
5454
this.emit("delete");
@@ -57,24 +57,29 @@ export class Watcher extends EventEmitter {
5757
this.emit("delete");
5858
});
5959

60-
const emitChange = lodashDebounce(
61-
(ctime) => this.emit("change", ctime),
62-
debounce,
63-
);
60+
const f = (ctime, stats) => {
61+
logger.debug("change", this.path, ctime);
62+
this.emit("change", ctime, stats);
63+
};
64+
const emitChange = debounce ? lodashDebounce(f, debounce) : f;
65+
6466
this.watcher.on("error", (err) => {
65-
L.debug("WATCHER error -- ", err);
67+
logger.debug("WATCHER error -- ", err);
6668
});
6769

6870
this.watcher.on("change", (_, stats) => {
6971
if (stats == null) {
70-
L.debug("WATCHER change with no stats (shouldn't happen)", { path });
72+
logger.debug("WATCHER change with no stats (shouldn't happen)", {
73+
path,
74+
});
7175
return;
7276
}
73-
emitChange(stats.ctime);
77+
emitChange(stats.ctime, stats);
7478
});
7579
}
7680

7781
close = async () => {
82+
logger.debug("close", this.path);
7883
this.removeAllListeners();
7984
await this.watcher.close();
8085
};

src/packages/project/sync/listings.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ export function registerListingsTable(table, query): void {
2727
await close_all_syncdocs_in_tree(path);
2828
};
2929

30-
const createWatcher = (path: string, debounceMs: number) =>
31-
new Watcher(path, debounceMs);
30+
const createWatcher = (path: string, debounce: number) =>
31+
new Watcher(path, { debounce });
3232

3333
const { project_id, compute_server_id } = query.listings[0];
3434

src/packages/server/purchases/subscription-renewal-emails.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ You have ${subs.length} ${siteName} ${plural(
111111
<ul>
112112
${subscriptionList.join("\n")}
113113
</ul>
114-
NOTE: You can easily cancel any subscription by clicking the link above without having to sign in to ${siteName}.
114+
NOTE: You can cancel a subscription via the link above without having to sign in to ${siteName}.
115115
116116
<br/><br/>
117117

src/packages/sync-client/lib/client-fs.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,8 @@ export class FileSystemClient {
220220
debounce?: number;
221221
}): Watcher => {
222222
const path = join(this.home, relPath);
223-
logger.debug("watching file", path);
224-
return new Watcher(path, interval, debounce, );
223+
logger.debug("watching file", { path, interval, debounce });
224+
return new Watcher(path, { interval, debounce });
225225
};
226226

227227
is_deleted = (_path: string, _project_id: string) => {

0 commit comments

Comments
 (0)