MB-55637: Added new error types in scorch

[capemox]

This error is called from the merger or persister when creation of a new zap file on disk is unsuccessful.

This helps filter out persistence errors and handle them in a different way compared to the other async errors.

[Copilot]

Pull request overview

This PR introduces a new error type AsyncPersistError to distinguish persistence failures from other async errors in the scorch index. The error type is used when segment persistence or merging operations fail to create zap files on disk, enabling specialized error handling for storage-related issues.

Key changes:

• Added AsyncPersistError struct to encapsulate persistence-related errors
• Updated persister and merger error handling to use the new error type

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
index/scorch/scorch.go	Defines the new AsyncPersistError type with error message field and Error() method
index/scorch/persister.go	Updates snapshot persistence error handling to use AsyncPersistError
index/scorch/merge.go	Updates merge error handling to use AsyncPersistError

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[CascadingRadium]

Hi @capemox, as part of this change can you consider if its worth checking whether we should be panicking in the following code:

bleve/index/scorch/persister.go

Line 1060 in bea7fc8

func (s *Scorch) removeOldData() {

When the cleanup routine tries to cleanup old zap files based on the snapshot information, if it sees that, somehow, a zap file mentioned in the old snapshot is missing on the disk (due to corruption/accidental deletes) it just panics.

This was seen many times in the previous cases and panics were logged. Since this is just cleanup of stale data files, we should not be panicking in that case is what I feel.

Perhaps we can have disk cleanup error as well?

type CleanupError struct {
    errMsg string
}

This will basically signal the caller to not panic as the index is in usable state (and just logs the fact that stale data files could not be cleaned up).

Also as defensive check maybe you can avoid panic in the following cases as well, if the config passed to the merge and persistor options, just fallback to using Default configs instead of panic. (This won't happen because we validate the options when opening/creating a new scorch object, but a good defensive check even then)

bleve/index/scorch/persister.go

Line 115 in bea7fc8

s.fireAsyncError(fmt.Errorf("persisterOptions json parsing err: %v", err))

bleve/index/scorch/merge.go

Line 46 in bea7fc8

mergePlannerOptions, err := s.parseMergePlannerOptions()

type ConfigError struct {
    errMsg string
}

I'll let the team weigh in on this. We can perhaps expose an interface ScorchError

[abhinavdangeti]

@capemox per our discussion let's see if we can pivot to embedding error context using %w within the errors and then use errors.Is(..) as opposed to type assertions downstream.

[abhinavdangeti]

@capemox 👍🏼