Is Syncthing v2 with very large folders possible ?

Support
201

That wouldn’t explain the total memory use not rising significantly. In fact unless connections are actively been closed it would make it rise faster by duplicating the cache among several connections.

It may not be a very well known problem because many standard (meaning out of application process) database systems share configurations between connections and a DB cache is always shared to avoid duplication. In fact I originally thought that SQLite would try to share the cache with shared memory between connections/processes unless it wasn’t supported (different systems using a network filesystem for storage or some OS without SHM support) but it doesn’t support it at all.

See for example : howto make pragma statements persistent, e.g. for foreign_keys checks · Issue #255 · mattn/go-sqlite3 · GitHub

202

I have an implementation that works with mattn/go-sqlite3 and doesn’t touch the modernc/sqlite driver. I’ve not yet cleaned the code (PRAGMAs are executed in the previous place too) but it works (I made sure of it by forcing the connections read-only with a PRAGMA in this code path and Syncthing didn’t like it one bit as expected…).

This works by creating two new driver implementations each adding a different hook at connect time, one is for the main DB and the other for the folder DB. I extended the database open methods to choose the driver. I believe there might be some opportunities to split the connection pool using this interface with different settings for each pool more suited for different parts of Syncthing.

For example it becomes trivial to allocate from :

  • a read-only pool by creating it with a driver using the read-only PRAGMA in its hook
  • pools with different cache sizes for the same DB to avoid wasting memory in parts that don’t benefit much from additional cache.

I’ve put it in a separate branch for now as I had to brute-force my way through database/sql, database/sql/driver and mattn/go-sqlite3 documentations to find a way to do this and I believe this could be polished a bit more.

I’ll push the branch to jtek/syncthing, I’ve restarted syncthing with it on our server after testing on my laptop and before writing this. I’m very curious about the perf and memory usage we’ll get with this.

203

Here it is, this includes all the previous work related to maintenance conversion to a slow walk of the tables working on chunks that are processed in <250ms :

204

Clearly it uses more memory so it confirms that once the connection-scope PRAGMA are moved in the connect phase they become useful.

The current RSS is around 24GiB and still rising instead of the 250-400MiB range I’ve seen it last run. I limited the concurrent connections to 4 simultaneous to limit the memory usage (in my configuration this means up to 7 * 4 + 2 * 4 = 36GiB). I’m not sure if it’s enough for most use cases though, there could be some large slowdowns if users hit the limit.

205

At the current rate of findings, the PR should be split up into more “digestible” parts.

  • connection hook
  • busy timeout
  • optimized PRAGMAs (temp_store, synchronous, … )
  • Cache size (maybe coordinate with @calmh who has a draft PR for configurable cache size?)
206

Yes. That depends on @calmh and he may not have time for this as I didn’t get a response on the cache_size coordination subject yet.

The busy_timeout change is just a single PRAGMA so I’m not sure it warrants a separate PR but that’s perfectly doable. Currently I suspect it could help the checkpointer do its job properly but that assumes that most of the time DB accesses are short which isn’t the case yet (the WAL grows during the “Preparing to Sync” phase probably so there’s no opportunity to checkpoint even with busy_timeout = 5000).

Optimized PRAGMAS are dependent on the connection hook to work properly so if they aren’t pulled in the same PR at least the PRAGMA one should depend on the connection hook.

The small chunk slow walk replacing the whole table or large random chunks cleanup is the core of the current PR and that could be isolated. This is what brings immediate relief for large folders.

The rest is a mix of bug fixing, attempts at simplifying (the checkpoint logic was painful to follow), tuning and preparing future changes.

The cache_size modification is not yet ready, the Scanning state is abnormally long this time which means even though setting it with the connection hook works it might need some tuning. Maybe it is because of the low MaxConns I changed which could slow things down (if all connections are used sqlx will wait for one to be available) but large caches could in some circumstances slow things down too. Testing with more connections and less cache today.

Additionally I’ll have to check if opening a new connection for tidy (which runs small incremental vacuums) is really needed too. Until there’s a clear need it’s better to reuse a connection.

207

I’ve been away skiing the past week, returning to normal operations soon.

208

Enjoy your time off :wink:

209

Apparently the dedicated connection for incremental vacuum/checkpoint truncate isn’t needed. At least it works properly on my laptop and our server without.

After reading the documentation again I think I found a simple way to avoid closing/opening connections.

I’ve forced the number of opened connections to remain stable with :

	sqlDB.SetMaxOpenConns(maxConns)
	sqlDB.SetMaxIdleConns(maxConns)

maxConns is only 6 for folders in the binary we use.

I’ll wait for our large folder to reach the “Preparing to Sync” state where more things happen to be sure but for now :

  • the memory usage is stable,
  • no new connection is opened (and so no connection closed and cache wasted),
  • I don’t see any slowdown in the UI (I had some with maxConns = 4).

The drawback is that the cache_size of each connection can’t be recovered so it probably uses more memory.

That said it doesn’t use much on common use cases (1.95MiB per connection) and if memory use must be optimized I believe different cache_size values for different uses would probably be the only viable option. Creating different drivers to use in openBase can make this work relatively easily with the pragmas_in_connect_hook branch.

210

The performance issue I have with the current code isn’t gone.

Scanning still takes an abnormally long time versus the usual 7 hours. I’ve waited 18 hours and it wasn’t finished. I have a theory about what causes this. If I’m right cache_size is not usable for large instances without working around the connection pool.

What I think is happening is that each time sqlx needs to work with the standard API it fetches an open connection from the pool or if there’s none available and there’s room for it, it creates one and adds it to the pool to use it. The problem is that this connection is most probably not the same one that was used the last time Syncthing was in the same section of its code and doing the same access patterns. Statistically, eventually all connections in the pool will have been used in all parts of the code using the default API (not a specific dedicated connection).

This would be mostly fine if the need for data from storage was sparse with a rate of data to load sufficiently slow that the caches will not evict previously loaded data soon. In the case of the maintenance, as long as the cache can keep the indexes in memory there’s a little room for other accesses that need to load a bit of data from the DB. From the maintenance access patterns it needs :

  • to load the whole index on removed as soon as it hits the GC for deleted files,
  • to slowly load the primary keys of file_names and file_versions and the corresponding indexes on the name_idx and version_idx of files to get what’s needed for the chunks being processed in the names/versions GC.
  • lastly and here lies the problem I believe : the blocks and blocklists cleanup. To find out what can be removed SQLite must scan the whole files table to find out if a blocklist_hash is unused. This probably means that a huge part if not the whole cache is replaced by content from the files table.

After a quick estimation (~30 million files, 100-150 bytes per row, 2 remote systems to sync with) on our largest folder the files table is probably on the order of 10GiB by itself (without indexes). Any cache size below this is mostly unusable and a waste of CPU. Even with a pool reduced to 8 connections a large enough cache_size is probably not doable with the 128GiB installed on this system.

This is consistent with the RAM usage I see, it rises very fast after the startup and I believe that after enough passes in the maintenance every connection ends up with its cache filed with files content.

In mainline the hash cleanup is done in a dedicated connection but I still believe the reason that was documented in the comments was wrong because there are no foreign keys involved. What is probably happening is that the query completely trashes the cache for any subsequent user of the connection.

Multiple connection pools with different cache_size values won’t be enough to fix this. I believe the only way to circumvent the fact that SQLite can’t share a cache between connections is forcing the performance critical parts of the code to use single long-lived connections dedicated to them with appropriate cache_size values for their job. For the hash cleanup if there isn’t enough RAM for cache_size to make it efficient a very low (the default for example) cache_size would be more efficient. It could even be a short lived one (as is the case in mainline), there’s a cost to establishing a connection but IIRC it isn’t much compared to the time needed for the query.

If we find several places using the same data we could share a connection (by making it a pool of one or putting the connection behind a Mutex). I’ll have to sit and think on this.

I’ll test my theory by either reverting my change in the hash cleanup to have short-lived dedicated connection or implement a long-lived one with a very large cache.

211

Pragmas that are per-connection require specific handling due to the pooling, yes, which is why they are usually provided as connection parameters by the underlying library. Our other connection-opening pragmas like journal mode etc are not of that variety.

The specific connection and foreign key disabling for block cleanup indeed looks unnecessary. It could possibly be beneficial if we wanted to run it with separate cache options or something.

212

I’ve seen this yes but I believe the connection parameters are a bit limited (ie: there isn’t a parameter for each and every PRAGMA that could be useful at least that seemed to be the case when I read the paramater parsing code in go-sqlite3). There’s no opportunity to fetch info from the database to adapt the PRAGMA like I did either (ie: probing the table sizes to choose an appropriate cache_size).

Although I didn’t work on the no C version IIRC I saw that there was a connect hook available with it too. This means that we could more easily share the settings using PRAGMAs instead of constructing two completely different connection URIs.

This is what I’m trying to do currently. I’m testing code using two separate pools :

  • the usual one but with MaxIdle set to MaxDBConns to avoid closing/reopening connections. Everything outside of maintenance is still done in it.
  • one pool with a single connection dedicated to the cleanups with cache_size = 40GiB (oversized but it will actually help me verify I can have a good estimate of the needed cache_size by monitoring the process RSS memory use as the rest of Syncthing doesn’t seem to allocate much memory).

I realized that as all cleanups in the periodic method are done in sequence there’s no reason to use more than one connection for all of them so better make them use the same one with a large cache to load all relevant data. By isolating this connection from the rest I protect its cache from potential cache trashing queries happening outside of the maintenance.

If this works properly (it should accelerate the maintenance/lower the load it generates) this could be integrated in the current PR with minimal changes (it is relatively easy to do without the connection hook).

213

I’ve got some interesting/surprising data out of this. Syncthing is currently using a total of 2.5GiB of RSS memory with this connection configuration.

I overestimated the files rows space usage. If I read the output of my manual select correctly the blocklist_hash is 256 bit not 512 bit and SQLite uses a varint for the integers. So the whole files table fitting in less than 2.5GiB with our configuration isn’t odd.

With a common pool (shared between maintenance and the rest of the code) the whole cache_size was used very quickly during the “scanning” state. This most probably means that there are queries doing full table scans outside the maintenance code and it is common (common enough to rapidly fill the cache of all the connections in the pool).

Note : as I write this our huge folder just reached the “Syncing” state. :tada:

The memory usage jumped to around 4GiB now that it is syncing (this is how I spotted the syncing state, I was monitoring the RAM usage).

Another thing : the checkpoints in our branch are working in the syncing state. I just saw the WAL being reduced from 2GiB to 8MiB. As I was writing this it grew to 24MiB before being reduced again to 8MiB.

It’s possible to integrate the maintenance dedicated 1-connection pool in our db_maintenance_performance branch : it is an easy modification and it really uses the cache efficiently (<2.5GiB covering all needs for a 60GiB database isn’t bad).

That said, the main benefit of the various cache_size changes was to help understand the access patterns. I don’t see a large performance boost from it (but our OS disk cache is mostly unused and available for Syncthing to use, it might bring a noticeable boost on systems where Syncthing isn’t the process making most of the disk accesses).

From what I see further gains might come from optimizing how Syncthing accesses the database in the Preparing to Sync and Scanning state. Based on cache usage, the Scanning state seems to make queries which need full table scans (which may reduce the scan performance) and Preparing to Sync blocks the checkpoints.

Preparing to Sync blocking checkpoints might not have much of an impact in the real world. We were in a very bad state with more than 2 months of lag trying to sync a 30 million files folder. With our current code, the maintenance should not be able to slow the scanning/preparing to sync states enough that it becomes a problem again for us.

Now there’s time to cleanup the code while the syncing progresses. There’s a smaller (~9 million files) folder waiting to sync where we can see the whole process play out again hopefully with code almost ready for a pull.

214

Before this topic auto-closes. I can report that the current code in the branch works for us : the second large folder is about to resync.

@calmh I regularly rebase on the main branch. so that it can more easily being pulled in. What should I do next to help my modifications being included ?

215

File a pull request with the changes, describing what what done and how that affects performance (and/or how to test that improved performance)?