Pebble pooled merger

[moodyjon]

Unbundled from PR: #43

See that for usage/testing.

Based on work by @BrannonKing in mem_pressure_try_2 and reduce_memory_pressure.

The stuff in 92d8b5f is something that seems unsafe which I found while debugging. The database was being corrupted, and I was seeing crashes in Change.Unmarshal() trying to read bytes that weren't there.

Eventually I figured out the problem was in the merger, and submitted 728ddc4

[coveralls]

Pull Request Test Coverage Report for Build 2435065658

• 44 of 44 (100.0%) changed or added relevant lines in 1 file are covered.
• 6 unchanged lines in 2 files lost coverage.
• Overall coverage increased (+0.05%) to 51.833%

---

Files with Coverage Reduction	New Missed Lines	%
btcec/signature.go	3	92.82%
peer/peer.go	3	76.16%

Totals
Change from base Build 2403229780:	0.05%
Covered Lines:	22452
Relevant Lines:	43316

---

💛 - Coveralls

[roylee17]

Is this still needed? From your comment, the culprit is the merger instead, right?

If the enc.WriteString(key) was not enough, the keySize = len(key) probably will won't save it, either.

[moodyjon]

If the enc.WriteString(key) was not enough, the keySize = len(key) probably will won't save it, either.

I'm just taking it for granted that truncating to 2 bytes is the right thing. Not familiar with the history of the comment.

The point of enc.WriteString(key[:keySize]) is to keep the 2-byte written length equal to length of the data actually written following it. If len(key) had to be truncated to fit in 2-bytes keySize, then the key data written should be keySize bytes. I'm concerned about what would happen if key was overly long. If there are SpentChildren elements following an overly long key (child1), then bytes of string child1 are interpreted as a length of the next key child2, and things go haywire unmarshalling child2.

You are correct though that I did NOT find something in the blockchain that triggers an actual problem here. It was the merger that had screwed up the data. I just started focusing here because the panic was in Unmarshal(), and it seemed like it always happened around block 400k. That was probably just because some pebble background merging activity tended to kick in at that point.

[moodyjon]

Might be better to use min(len(key), Uint16Max) so the most bytes possible are preserved. ...Instead of len(key) = 2^16 leading to 0 bytes written, etc.

[roylee17]

I see.

I had a quick glance through the codebase, and it does seems that we don't have checker for the name length (not 100% sure)
.
But I think we should put that checker closer to the input, and relief the rest of code path from the worries.

[roylee17]

If I read the graph right, it does cut 40GB of memory allocation (from LoadChanges and Pebble background compaction) during that 1hr.

Before:

After:

The improvements should reflect in faster sync time after all.

I'm having full syncs (before and after the changes) from scratch over the weekend on a dev server.
Looking forward to it.

[moodyjon]

It might only affect CPU usage during sync. Or it might only affect the "Building entire claimtrie in RAM" phase.

For my runs, I think the peer would only provide blocks at a certain rate and that controlled the overall time.

[BrannonKing]

The key size is for names, which are limited in the mempool to 256 (or 255) bytes. Even if we expand them to the worst possible length with our normalization, they won't be longer than 4x that. Hence, two bytes will always be long enough for the length of a name.

When I originally coded this I couldn't see any difference in runtime (wall-clock) performance.

[roylee17]

Ran some tests over the weekend, and it turned out the sync time improves only slightly 13h 54m vs 13h 38m while RamTrie rebuild time significantly reduced, this is very helpful to the development

Sync time

Before:

2022-06-06 06:37:12.838 [INF] MAIN: Version 0.0.0-local.0+15191b7ede02f4c052a2a61c55c8ef7a95d8e134
2022-06-06 20:31:12.230 [INF] SYNC: Processed 63 blocks in the last 10s (10282 transactions, height 1163018, 2022-05-20 23:52:46 +0000 UTC)

After:

2022-06-05 16:22:32.183 [INF] MAIN: Version 0.0.0-local.0+c4e7528a6a87e37d631925ad19e8a78675609fa0
2022-06-06 06:00:58.141 [INF] SYNC: Processed 94 blocks in the last 10.2s (12943 transactions, height 1163063, 2022-05-21 01:52:52 +0000 UTC)

(Re) Startup time

Before:

2022-06-06 20:38:43.435 [INF] MAIN: Version 0.0.0-local.0+15191b7ede02f4c052a2a61c55c8ef7a95d8e134
2022-06-06 20:45:37.155 [INF] CHAN: Loading block index...

After:

2022-06-06 20:47:34.503 [INF] MAIN: Version 0.0.0-local.0+c4e7528a6a87e37d631925ad19e8a78675609fa0
2022-06-06 20:51:05.282 [INF] CHAN: Loading block index...