rename pindex functions to not include import#8098
Conversation
prepare for use of functions to be shared by resync and import
There was a problem hiding this comment.
Pull request overview
This PR prepares the pindex helper functions for use by both resync and import by renaming them to remove the "import"-specific naming. It also improves error messages and updates log statements.
Changes:
- Renamed private helpers (
registerImportPindexImplMutex→registerPindexImplMutex,getListenerImportDest→getCbgtDest,newImportPIndexImpl→newPIndexImpl) to be generic - Improved the error message when unmarshalling
SGFeedIndexParamsto include the problematic value (withbase.MD()wrapping) - Downgraded the "No dest found for indexParams" log from
InfofCtxtoDebugfCtx
| if err != nil { | ||
| // This error can occur when a stale index definition hasn't yet been removed from the plan (e.g. on update to db config) | ||
| base.InfofCtx(ctx, base.KeyDCP, "No importDest found for indexParams - usually an obsolete index pending removal. %v", err) | ||
| base.DebugfCtx(ctx, base.KeyDCP, "No dest found for indexParams - usually an obsolete index pending removal. %v", err) |
There was a problem hiding this comment.
The log level for "No dest found for indexParams" was downgraded from InfofCtx to DebugfCtx. While the comment says this is "usually an obsolete index pending removal", it is still an error path where err != nil. Downgrading to Debug may make it harder to diagnose real issues in production environments where debug logging is not enabled. At minimum, if this case is truly expected during normal operation, the comment or log message should clarify when it represents an actual problem versus a transient state.
There was a problem hiding this comment.
This was intentional.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
RIT3shSapata
left a comment
There was a problem hiding this comment.
shouldn't the filename also be changed?
I'll change the filename now, the reason I didn't originally is that changing the filename will make the diff in github hard to read. |
prepare for use of functions to be shared by resync and import