(CC: @stamblerre @alandonovan)

I’ve also been seeing this more frequently lately and would appreciate a fix.

I'm actually not familiar with this codebase, but I can look into it.

@stamblerre I was looking around this code for an unrelated search proposal I'm working on so I figured I'd post some notes to make myself useful in case my proposal falters.

I believe the message is set here:

if ts := c.FSModifiedTime(); timestamp.Before(ts) {
    // The index is older than the latest file system change under godoc's observation.
    result.Alert = "Indexing in progress: result may be inaccurate"
}

It looks like a reindex happens every 5 minutes: initialization (not set), usage (use default). This is actually surprising to me since I recall getting the indexing message multiple times in a 10 minute window in what I remember to be off hours. So it's hard to believe that the filesystem is actually changing. Looks like fsModified is only set by invalidateIndex which is called by initFSTree which is called by RunIndexer in every iteration of the forever loop. This tells me that it's not actually based on when fs changes. In any case, you can get more data and maybe prove me wrong by running in verbose mode and seeing timestamps from UpdateIndex's logs and seeing if there were any commits between last index and the time you get the indexing alert.

Otherwise, we should prefer to serve stale data instead of an unhelpful error.

That might just be removing the if code above.

If indexing usually finishes quickly, we should just wait to serve the page until it's ready.

Maybe use a channel which UpdateIndex/RunIndexer sends on and then if it sees index in progress Lookup selects on that channel and a short timeout .

This tells me that it's not actually based on when fs changes.

That is correct. There was a separate issue (#7524) which tracked changing this to respond to filesystem changes. But that was closed deciding that controlling the index_interval with the flag is good enough.

That might just be removing the if code above.

Yes, looks like it. I agree that we should just return stale data instead of returning an error.

But then, this is for golang.org. So when it is deployed, it just indexes the standard library. There is no user code here. So, essentially nothing ever changes and we are just re-indexing anyways.

I would suggest setting the index_interval to 1 week or something. So that it effectively becomes a non-issue.

/cc @andybons

/cc @broady who was just playing around with godoc.

Was going through the godoc flags today and realised that a negative index_interval just indexes once, effectively disabling indexing after that. I think we should set that for golang.org.

Closing. Please re-open if you see this on golang.org again.

Indexing happens as part of the deployment process, so this message shouldn't ever be seen.

Edit: shouldn't have been so hasty to close. There's a minor race condition.

RunIndexer is called in a goroutine, so between the first call to c.UpdateIndex() and the sever starting, users will see this message.

With the move to flex, this won't be seen almost at all in practice, but on the old deployment which was OOMing, basically only a single request would be handled before OOM so the chances of seeing this error was high.

@broady - I mean it's not a race condition "per se". Because the access is mutex protected.

Still, this c.readIndex should happen only once when godoc starts. So even if users see the message, it should be for a very thin window, i.e. the time it takes to read the index file. The issue was about "frequent" indexing failures, which I don't think should be happening now.

I don't see any obvious way around this, unless you want to make c.UpdateIndex() synchronous ?

It's a race between c.UpdateIndex and the first incoming request to the search page.

Yes, in production mode, UpdateIndex should be synchronous (i.e., happen before incoming requests are handled).

That said, it doesn't really matter very much (see the last para in my previous comment).

Change https://golang.org/cl/148998 mentions this issue: cmd/godoc: start RunIndexer synchronously

@broady, do we still ship a pre-built index for golang.org on Flex?

Change https://golang.org/cl/150685 mentions this issue: [release-branch.go1.11] cmd/godoc: start RunIndexer synchronously when index is present