Proposition for two new events and a new REST function

Feature
1

I’d like to propose two additional events available via the events API: LocalIndexUpdateTriggered and RemoteIndexUpdateTriggered.

I still don’t understand how st works, so pardon me if I talk nonsense, but I imagine that LocalIndexUpdateTriggered should be generated as soon as it’s clear to st that we are going to update the local index. At the same time this information should be sent to other devices so that they can generate RemoteIndexUpdateTriggered.

In terms of generated traffic this causes only a minuscule increase.

My direct reason for that is that this allows for a little bit more precise monitoring. For example, I think it’s established that “st type software” :slight_smile: is used also in the following situation: I ssh to a remote host which runs st, I copy data to ./Sync on remote host, I wait for the data to be synced. Now if my local host gets RemoteIndexUpdateTriggered say 3 seconds before RemoteIndexUpdated then for example stiko can inform me about it 3 seconds earlier than now, saving me 3 seconds of wondering whether everything’s ok.

Imagine that: you log in to a remote host, you start copying something to ./Sync, and 0.1s after copying has finished your notification icon tells you syncing has started. Gives you really nice feeling in your tummy.

An indirect reason could be that this will at some future point also allow to speed things up for the syncthing application: if there are things which must be done after RemoteIndexUpdated is received, but which do not directly need seeing the remote index (like perhaps creating the local index?), then this can also be done 3 seconds earlier.

A proposition for a new REST function is unrelated to the above, but it’s also meant as a helper for stiko type application. When stiko starts it’s sometimes hard to know where we are in terms of st operation. For example, conceivably, we might be in the middle of syncing which goes very slowly, and no sensible events are generated for good couple of seconds or even more, and stiko just can’t correctly tell the user what is going on.

So it’s natural that stiko asks st about various things accesible via REST. But in the mean time maybe new events are generated? And so on. I’m sure one can deal with it with current means (although I don’t quite know how to really cover all corner cases). But it would be nice to have a REST call: just_tell_me_everything(), which would tell the config, connected hosts, all known information about connected hosts (like their most recent FolderCompletion), and so on, and the id number N such that all that information is current as of event N. Then stiko would also at its start know where we are, and that we should just start listening from event N+1.

This reminds me about the third issue: according to the docs, id is “A monotonically increasing integer”. Doesn’t it go to zero sometimes?

2

The reason you have the delay is most likely the scanning delay, and not the amount of time it takes for a remote fragment to arrive.

Remote index updates are limited to some amount which I think is 200KB, so unless you are on a satellite link, sending 200KB should be instant.

3

As for the second part, you can get the last 1000 events from the buffer and process that, even if you started just now. You can make multiple calls to get the state that you are interested in, I don’t think the process is prone to races, hence not sure what’s the argument for tell_me_everything.

Events use long polling, so you can use a second thread to ask for something else in the meantime.

I am not sure what ID you are referring to.

4

As for the new events LocalIndexUpdateTriggered and RemoteIndexUpdateTriggered, I stand corrected and revoke the proposition (I did some more tests and indeed my connection is the only thing which slows down the notification).

By “id” I meant the “id” field of an event, as given via localhost:8384/rest/events?since=0

As for the REST function tell_me_everything - there are situations when 1000 is not enough, but even if it was, my point is that st “knows its state” anyway, so it should be very easy to add such a function to the REST API. The alternative is having inside every notification app an event parser which allows to deduce the current st state given the event history, but this seems to me not the right way to proceed (since, again, st could just tell its state because it knows it - it could be an expensive call, but avery notification app needs to call it just once).

5

This is fixed in the next release.

This already exists, but as I said, consists of multiple calls. Events are just updates to the base state. Synctrayzor et all already do this, they get the initial state using the normal calls, and then mutate the state using events.

6

And since events are just updates to the base state, when I query, say, http://localhost:8384/rest/system/connections, it would be nice to also get the event_id such that the information I get about connections is correct at event_id. This is my main point.

Additionally - because it’s so easy for st - why not provide all the info st has + the information at which event_id was this info correct in a new tell_me_everything call.

7

Right now Syncthing doesn’t really know this, internally, although in some cases it could. For example the place that does the generating of connection events could remember the event id it generated and store this somewhere. But I’m not sure of the value of doing so, i.e. what does it enable you to do as an API consumer that you couldn’t otherwise?

For other things it’s trickier. For example when querying the current database state we grab a read transaction and calculate the necessary info. But one “index received” event may well be handled with multiple database transactions, so the data returned from the query can be a state from in between event IDs, so to speak.

That doesn’t matter much for the intended use case (the GUI) as it only uses this information for he initial view on load. Soon thereafter it’ll get an event describing the new database state.

8

Basically, my point is that it’s an already solved problem, as other wrappers work without the need of any extra calls, so I find it hard to justify the request, as all it does is just merges output of multiple calls.

You can get the last even id by getting the last event, make all calls, get all events since the event you checked to make sure you haven’t missed anything.

9

Ok, if it’s difficult for st to do it as well then indeed there’s not much point. Somewhat stupidly only now I’ve noticed that all the rest calls seem to give time stamps so it seems I can actually use this to calculate the base state with 100% accuracy.

Still, it would be nice if st could expose, for every connected peer, their most recent reported folder completion - this is a problem which might even appear in practice: my impression is that it could happen that when a notification app starts, that info might already be not in the buffer, and on the other hand asking for folder completions via rest if there are 1000 peers is very costly.

so I check last event id, it’s 100, I make all calls, now last event is 10000 because there was a shitload of traffic, and since the calls don’t tell me if they were correct at 267 or 9945, I still have to parse all the events to get the base state with 100% accuracy.

As calmh notes it doesn’t really matter because in practice there are soon gonna come events which will tell me what I need. All I was trying to say is that it would be nice to have a cleaner solution for computing the base state.

Audrius, you seem to be claiming that the current wrappers would work no matter what, let’s say with 10000 peers, some very fast connections, some very unreliable ones. In other words, you seem to claim that at least some current wrappers work correctly not only in practice, but also in the purely theoretical sense of reproducing at any given moment the exact state of st (or as much of it as they are interested in) up to, say 0.1s delay - is that correct, would you sign your name under it? :slight_smile:

10

Why? What makes you think this is the case?

That is very unlikely given you’ll hit scalability issues elsewhere before you can generate enough events in such a short period of time. Even if that is the case, you can just repeat the sequence until you fall within the last 1000 events.

There is already a way to compute the base state, and it’s just getting it from the rest calls. If you are fine with updating the state once every N seconds, you can just ignore the events interface and call the rest calls.

What I am saying, is that currently wrappers do not have any issues with whatever the users are doing, because if they would have problems, we’d fix them. I am suggesting not to waste any effort for problems that don’t exist, or exist in theory. I am sure slow GUI updates will be the least of your problems at the scale of 10000 nodes, as there are other parts in syncthing which will blow up first.

Even if you have a 10000 nodes, you will still have to make 3-5 calls to get the whole information.

If you are not satisfied with performance, the SLA’s or the support you should ask for a refund. Nobody will sign anything, and you trying to apply some sort of pressure just makes you look bad.

11

That would be the rest api documentation.

This is a valid point. The pedant in me would rather have an api which (under some assumptions) guarantees that notification apps work correctly, but of course it’s your choice as developers.

12

That is outdated, it’s no longer the case, as we do that in the background every 2 seconds and just provide the cached version when asked.

It does that now, under the assumption that you will not roll more than 1000 events between the calls.

13

Well, that changes quite a lot (and expemlifies the need for reliable ways to establish common base state, doesn’t it). As of which version this is the case? Using web browser with v0.11.25 feels like this call is actually very slow.

14

Sounds like I should really try v0.12, that calls make the GUI on slow devices more or less unusable. Is the same valid for /rest/db/status?

15

It’s already the case in the current latest version, unless there is a bug.