Don't prune ancient state when instantiating a Client

[dvdplm]

This is possibly a controversial PR.
Before this PR, a call to Client::new() will delete all state below the --pruning-history threshold. This is side-effectey and surprising and I don't think it is necessary; I think it is better if Client::new() avoids touching the database.
For commands like e.g. parity snapshot, kicking off a pruning run before starting the snapshot can be problematic as state might be deleted before we even start; for other commands like import, export, or reset, pruning the state before executing the command seems unneccesary and slow.

After this the client will prune ancient state as a side effect of importing new blocks (in commit_block()), which I think is fine.

[debris]

it seems reasonable

[0x7CFE]

How it was tested?

[dvdplm]

How it was tested?

Not 100% sure I understand what you mean by "testing" here, but you are right that I am uncertain about what consequences of skipping pruning-on-start are. I am not sure why we prune at startup in the first place, possibly to reduce memory usage for the case the user has changed the pruning history settings between restarts?
Do you, or anyone, have any suggestion on how to test this change?

[0x7CFE]

Do you, or anyone, have any suggestion on how to test this change?

Not at the moment. Have you tried syncing the node on a clean or populated db? If that works OK, then I would probably agree that this change worth it.

Do you have any estimations on how much will PR save in time on a real world scenario?

[dvdplm]

The pruning only runs at startup so I don’t expect any performance impact at all on nodes that are already running tbh. For nodes running with a big pruning history (1000 or above), startup is noticeably faster which impacts short lived commands such as import/export etc.

[0x7CFE]

Fair enough.