Discover: Selected field shows with wrong icon when switching from unmapped to mapped

[flash1293]

Kibana version: main

Describe the bug: When a selected field changes from unmapped to mapped in a data view, it is shown with the wrong icon even after a refresh. The field needs to be removed and re-added for the icon to update.

Steps to reproduce:

• Create a new index:

PUT my-index
{
  "mappings": {
    "dynamic": false,
    "properties": {}
  }
}

POST my-index/_doc
{
  "test": "hello world"
}

• Create data view for the index
• Go to Discover and add the test field:
  
• Recreate the index and map the field as keyword:

DELETE my-index

PUT my-index
{
  "mappings": {
    "dynamic": false,
    "properties": {
      "test": {
        "type": "keyword"
      }
    }
  }
}

POST my-index/_doc
{
  "test": "hello world"
}

• Reload discover - selected field is still shown with question mark icon in the field list but not in the table
  

Expected behavior:

Field is consistently shown as keyword field everwhere

Screenshots (if relevant):

Errors in browser console (if relevant):

Provide logs and/or server output (if relevant):

Any additional context:

[elasticmachine]

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

[davismcphee]

I believe this is caused by data view field list caching: #168910. We use a stale while revalidate caching strategy which means field list results can be outdated for a single request when fields change while the latest results are fetched in the background. This happens at the browser cache level so we don't currently have programatic control over the caching unfortunately.

The issue resolves itself by continuing to use Kibana since fields are frequently requested, so results aren't outdated for long. For example, in your provided test case the fields are synced after refreshing Discover a second time or after executing more searches. It was a tradeoff we made when implementing caching since we believed it was more important for users to get field list results fast than results to be synced immediately.

[flash1293]

Hmm, but it shows the right icon in "Available fields" right below, so it does know about the updated type in the browser memory.

[flash1293]

If the UI would consistently show the field as unmapped I agree that it is OK, I was mostly hung up by the inconsistency in the field list / data grid

[kertal]

one addon, it's a secondary request, uncached, determining if it's available or not. opening the browser, and deactivating caching, could be used to figure out if it's caching that causing it. (Looks like this on the first sight / read)

[davismcphee]

As @kertal mentioned, it's because there are two separate fields requests -- the first one when the data view is initialized, and the second one from the field list sidebar directly. The first one is served from the cache while revalidation is triggered in the background, and the second one occurs after revalidation, so it contains the updated results:

Also as @kertal mentioned, this goes away when deactivating the browser cache. But since we're getting the updated fields from the sidebar request in this case anyway, we could likely figure out a way to sync those updated results back to the data view so it gets the updated fields too. I'm just not immediately sure what that effort would involve so we'd need to investigate more.

[flash1293]

Not a super critical one, I just noticed it when working on the streams effort which will make going from unmapped to mapped much more common

[kertal]

We could use the secondary request to update mappings of the primary request, if fields have changed ... or to trigger a reload of the primary one. However this is not a low hanging fruit. I'd treat this an potential UX enhancement for this case, not as a bug, as it works like expected.

[flash1293]

Why are we doing two requests anyway? That seems inefficient, right? Wouldn't it make sense to pull the state management of this thing a level higher and serve all consumers with it?

[flash1293]

I'd treat this an potential UX enhancement for this case, not as a bug, as it works like expected.

I disagree here - if it would just be outdated but consistent in the UI, then yes, it's just a UX enhancement. But if it's inconsistent on a single page, that's a bug. Doesn't have to change the priority though, I'll leave that up to you. I expect with the streams effort progressing we'll find more things like this because we are using Elasticsearch a bit different than usual, so we hit different edge cases (like elastic/elasticsearch#117544 which surfaced this way as well)

[kertal]

Why are we doing two requests anyway? That seems inefficient, right? Wouldn't it make sense to pull the state management of this thing a level higher and serve all consumers with it?

There's 1 initial request when selectiing the data view. The secondary request for available fields is a follow up of every search request. I do agree it would be more efficient if fields where returned with the search request. And that's what happens using ES|QL

I disagree here - if it would just be outdated but consistent in the UI, then yes, it's just a UX enhancement. But if it's inconsistent on a single page, that's a bug. Doesn't have to change the priority though, I'll leave that up to you. I expect with the streams effort progressing we'll find more things like this because we are using Elasticsearch a bit different than usual, so we hit different edge cases (like elastic/elasticsearch#117544 which surfaced this way as well)

Thx for raising awareness for this, fine with that, let's treat it as a bug with low priority 👍

[davismcphee]

Why are we doing two requests anyway? That seems inefficient, right? Wouldn't it make sense to pull the state management of this thing a level higher and serve all consumers with it?

The first is unfiltered for all fields in the data view when instantiated (this is cached for subsequent requests), and the other is from the field list with an index_filter containing the time range to narrow field list results (always uncached). I'm not sure pulling this up into a single request would be feasible, but I'm not outright opposed to the idea if it could work.

if it would just be outdated but consistent in the UI, then yes, it's just a UX enhancement. But if it's inconsistent on a single page, that's a bug.

I don't think it's just a single page, the data view field list will always be one request out of date once cached across all of Kibana where data views are used. The inconsistency here is that Discover attempts to remedy this in its own field list when it detects a mismatch, but there's a flaw in this logic, so I still think considering it a bug is appropriate.

I dug into it more today and I overlooked something originally (mainly what makes this a bug/unexpected): We are actually already trying to detect this in Discover and correct it when we notice the field list is outdated. We typically expect one /fields request and one /_fields_for_wildcard request, but in your scenario we actually see two /fields requests and one /_fields_for_wildcard request. The second /fields request is triggered to update the data view field list when we notice a mismatch in mapped fields after receiving the uncached _field_for_wildcard response:

kibana/packages/kbn-unified-field-list/src/services/field_existing/field_existing_utils.ts

Lines 60 to 67 in f8860e9

	// take care of fields of existingFieldList, that are not yet available
	// in the given data view. Those fields we consider as new fields,
	// that were ingested after the data view was loaded
	const newFields = existingFieldList.filter((field) => !dataView.getFieldByName(field.name));
	// refresh the data view in case there are new fields
	if (newFields.length) {
	await dataViewsService.refreshFields(dataView, false, true);
	}

This is how the available fields section gets updated after refreshing even though the data view field list is served from the cache. It looks like there's just a bug in the logic since selected fields are handled differently than the other sections. I'm assuming we can likely fix this, although it won't address the broader issue of cached data view field lists being outdated for a single request. Relevant PR for reference: #172329.