Skip to content

Commit 29ffa00

Browse files
authored
Merge pull request #4758 from bbc/t3214-mappings-exclude-vectors
T3214 mappings exclude vectors
2 parents cdf30a5 + b7fdc57 commit 29ffa00

8 files changed

Lines changed: 71 additions & 42 deletions

File tree

common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfigWithElastic.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ class CommonConfigWithElastic(resources: GridConfigResources) extends CommonConf
1313
),
1414
url = string("es6.url"),
1515
shards = string("es6.shards").toInt,
16-
replicas = string("es6.replicas").toInt
16+
replicas = string("es6.replicas").toInt,
17+
includeDenseVectorMappings = booleanOpt("es.includeDenseVectorMappings").getOrElse(true)
1718
)
1819

1920
private val persistenceIdentifier = string("persistence.identifier")

common-lib/src/main/scala/com/gu/mediaservice/lib/elasticsearch/ElasticSearchClient.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ trait ElasticSearchClient extends ElasticSearchExecutions with GridLogging {
3737

3838
def shards: Int
3939
def replicas: Int
40+
def includeDenseVectorMappings: Boolean
4041

4142
lazy val client = {
4243
logger.info("Connecting to Elastic 8: " + url)
@@ -146,7 +147,7 @@ trait ElasticSearchClient extends ElasticSearchExecutions with GridLogging {
146147
"to remove the need for these: " + nonRecommendenedIndexSettingOverrides)
147148

148149
createIndex(index).
149-
mapping(Mappings.imageMapping).
150+
mapping(Mappings.imageMapping(includeDenseVectorMappings)).
150151
analysis(IndexSettings.analysis).
151152
settings(nonRecommendenedIndexSettingOverrides).
152153
shards(shards).
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
package com.gu.mediaservice.lib.elasticsearch
22

3-
case class ElasticSearchConfig(aliases: ElasticSearchAliases, url: String, shards: Int, replicas: Int)
3+
case class ElasticSearchConfig(aliases: ElasticSearchAliases, url: String, shards: Int, replicas: Int, includeDenseVectorMappings: Boolean = true)

common-lib/src/main/scala/com/gu/mediaservice/lib/elasticsearch/Mappings.scala

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import play.api.libs.json.{JsObject, Json}
88

99
object Mappings {
1010

11-
val imageMapping: MappingDefinition = {
11+
def imageMapping(includeDenseVectorMappings: Boolean = true): MappingDefinition = {
1212

1313
// Non indexed fields stored as keywords can still participate in exists / has queries
1414
def filemetaDataStringsAsKeyword: DynamicTemplateRequest = {
@@ -75,8 +75,7 @@ object Mappings {
7575
leasesMapping("leases"),
7676
collectionMapping("collections"),
7777
esInfoMapping("esInfo"),
78-
embeddingMapping("embedding"),
79-
)
78+
) ++ (if (includeDenseVectorMappings) Seq(embeddingMapping("embedding")) else Seq.empty)
8079
)
8180
}
8281

dev/script/generate-config/service-config.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ function getKahunaConfig(config){
146146
|]`;
147147

148148
const permissionsConfig = stripMargin`
149-
|usePermissionsFilter=true
149+
|usePermissionsFilter=false
150150
|permissionsDefault="allPermissions"
151151
`;
152152

@@ -191,6 +191,7 @@ function getMediaApiConfig(config) {
191191
|es6.url="${config.es6.url}"
192192
|es6.shards=${config.es6.shards}
193193
|es6.replicas=${config.es6.replicas}
194+
|es.includeDenseVectorMappings=true
194195
|quota.store.key="rcs-quota.json"
195196
|security.cors.allowedOrigins="${getCorsAllowedOriginString(config)}"
196197
|search.fuzziness={
@@ -225,6 +226,7 @@ function getThrallConfig(config) {
225226
|es6.url="${config.es6.url}"
226227
|es6.shards=${config.es6.shards}
227228
|es6.replicas=${config.es6.replicas}
229+
|es.includeDenseVectorMappings=true
228230
|metrics.request.enabled=false
229231
|`;
230232
}

media-api/app/lib/elasticsearch/ElasticSearch.scala

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ class ElasticSearch(
7575
lazy val url = elasticConfig.url
7676
lazy val shards = elasticConfig.shards
7777
lazy val replicas = elasticConfig.replicas
78+
lazy val includeDenseVectorMappings = elasticConfig.includeDenseVectorMappings
7879

7980
private val SearchQueryTimeout = FiniteDuration(10, TimeUnit.SECONDS)
8081
// there is 15 seconds timeout set on cluster level as well
@@ -180,18 +181,23 @@ class ElasticSearch(
180181

181182
def knnSearch(queryEmbedding: List[Float], k: Int, numCandidates: Int, filterOpt: Option[Query])
182183
(implicit ex: ExecutionContext, logMarker: LogMarker): Future[SearchResults] = {
183-
val knn = Knn("embedding.cohereEmbedV4.image", filter = filterOpt)
184-
.queryVector(queryEmbedding.map(_.toDouble))
185-
.k(k)
186-
.numCandidates(numCandidates)
184+
if (!includeDenseVectorMappings) {
185+
logger.warn(logMarker, "knnSearch called but includeDenseVectorMappings=false, returning empty results")
186+
Future.successful(SearchResults(Nil, total = 0, extraCounts = None))
187+
} else {
188+
val knn = Knn("embedding.cohereEmbedV4.image", filter = filterOpt)
189+
.queryVector(queryEmbedding.map(_.toDouble))
190+
.k(k)
191+
.numCandidates(numCandidates)
187192

188-
val searchRequest = ElasticDsl.search(imagesCurrentAlias)
189-
.knn(knn)
190-
.size(k)
193+
val searchRequest = ElasticDsl.search(imagesCurrentAlias)
194+
.knn(knn)
195+
.size(k)
191196

192-
executeAndLog(withSearchQueryTimeout(searchRequest), "knn search").map { r =>
193-
val imageHits = r.result.hits.hits.map(resolveHit).toSeq.flatten.map(i => (i.instance.id, i))
194-
SearchResults(hits = imageHits, total = imageHits.length, extraCounts = None)
197+
executeAndLog(withSearchQueryTimeout(searchRequest), "knn search").map { r =>
198+
val imageHits = r.result.hits.hits.map(resolveHit).toSeq.flatten.map(i => (i.instance.id, i))
199+
SearchResults(hits = imageHits, total = imageHits.length, extraCounts = None)
200+
}
195201
}
196202
}
197203

@@ -271,15 +277,20 @@ class ElasticSearch(
271277
implicit ex: ExecutionContext,
272278
logMarker: LogMarker
273279
): Future[SearchResults] = {
274-
val queryEmbeddingDouble: List[Double] = queryEmbedding.map(_.toDouble)
275-
276-
for {
277-
maxScore <- fetchMaxBm25Score(query, filterOpt)
278-
searchRequest = makeHybridSearchRequest(query, queryEmbeddingDouble, k, numCandidates, vecWeight, maxScore, filterOpt)
279-
result <- executeAndLog(withSearchQueryTimeout(searchRequest), "hybrid search")
280-
} yield {
281-
val imageHits = result.result.hits.hits.map(resolveHit).toSeq.flatten.map(i => (i.instance.id, i))
282-
SearchResults(hits = imageHits, total = imageHits.length, extraCounts = None)
280+
if (!includeDenseVectorMappings) {
281+
logger.warn(logMarker, "hybridSearch called but includeDenseVectorMappings=false, returning empty results")
282+
Future.successful(SearchResults(Nil, total = 0, extraCounts = None))
283+
} else {
284+
val queryEmbeddingDouble: List[Double] = queryEmbedding.map(_.toDouble)
285+
286+
for {
287+
maxScore <- fetchMaxBm25Score(query, filterOpt)
288+
searchRequest = makeHybridSearchRequest(query, queryEmbeddingDouble, k, numCandidates, vecWeight, maxScore, filterOpt)
289+
result <- executeAndLog(withSearchQueryTimeout(searchRequest), "hybrid search")
290+
} yield {
291+
val imageHits = result.result.hits.hits.map(resolveHit).toSeq.flatten.map(i => (i.instance.id, i))
292+
SearchResults(hits = imageHits, total = imageHits.length, extraCounts = None)
293+
}
283294
}
284295
}
285296

scripts/src/main/scala/com/gu/mediaservice/scripts/EsScript.scala

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -197,15 +197,19 @@ object Reindex extends EsScript {
197197
object UpdateMapping extends EsScript {
198198

199199
def run(esUrl: String, extraArgs: List[String]): Unit = {
200+
val includeDenseVectors = !extraArgs.contains("--no-dense-vectors")
201+
val remainingArgs = extraArgs.filterNot(_ == "--no-dense-vectors")
200202

201-
object MappingsClient extends EsClient(esUrl) {
203+
println(s"includeDenseVectorMappings: $includeDenseVectors")
204+
205+
object MappingsClient extends EsClient(esUrl, includeDenseVectors) {
202206
def updateMappings(specifiedIndex: Option[String]): Unit = {
203207
val index = specifiedIndex.getOrElse(imagesCurrentAlias)
204208
println(s"Updating mapping on index: $index")
205209

206210
val result = client.execute {
207211
putMapping(Indexes(index)) as {
208-
Mappings.imageMapping.properties
212+
Mappings.imageMapping(includeDenseVectors).properties
209213
}
210214
}.await
211215

@@ -219,11 +223,11 @@ object UpdateMapping extends EsScript {
219223

220224
}
221225

222-
MappingsClient.updateMappings(extraArgs.headOption)
226+
MappingsClient.updateMappings(remainingArgs.headOption)
223227
}
224228

225229
def usageError: Nothing = {
226-
System.err.println("Usage: UpdateMapping <ES_URL>")
230+
System.err.println("Usage: UpdateMapping <ES_URL> [<index_name>] [--no-dense-vectors]")
227231
sys.exit(1)
228232
}
229233
}
@@ -422,7 +426,7 @@ abstract class EsScript {
422426
run(esUrl, extraArgs)
423427
}
424428

425-
class EsClient(val url: String) extends ElasticSearchClient {
429+
class EsClient(val url: String, val includeDenseVectorMappings: Boolean = true) extends ElasticSearchClient {
426430
override def imagesCurrentAlias = esImagesAlias
427431
override def shards = esShards
428432
override def replicas = esReplicas

thrall/app/lib/elasticsearch/ElasticSearch.scala

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ class ElasticSearch(
4141
lazy val url: String = config.url
4242
lazy val shards: Int = config.shards
4343
lazy val replicas: Int = config.replicas
44+
lazy val includeDenseVectorMappings: Boolean = config.includeDenseVectorMappings
4445

4546

4647
def migrationAwareUpdater[REQUEST, RESPONSE](
@@ -78,9 +79,13 @@ class ElasticSearch(
7879
executeAndLog(request, s"Setting migration info on image id: ${imageId}")
7980
}
8081

82+
private def prepareImageJson(json: JsObject): JsObject =
83+
if (includeDenseVectorMappings) json
84+
else json - "embedding"
85+
8186
def directInsert(image: Image, indexName: String)(implicit ex: ExecutionContext, logMarker: LogMarker): Future[ElasticSearchInsertResponse] =
8287
executeAndLog(
83-
indexInto(indexName).id(image.id).source(Json.stringify(Json.toJson(image))),
88+
indexInto(indexName).id(image.id).source(Json.stringify(prepareImageJson(Json.toJson(image).as[JsObject]))),
8489
s"ES6 indexing image ${image.id} into index '$indexName'"
8590
).map(indexResponse =>
8691
ElasticSearchInsertResponse(indexResponse.result.index)
@@ -92,14 +97,14 @@ class ElasticSearch(
9297

9398
// On insert, we know we will not have a lastModified to consider, so we always take the one we get
9499
val insertImage = image.copy(lastModified = Some(lastModified))
95-
val insertImageAsJson = Json.toJson(insertImage)
100+
val insertImageAsJson = prepareImageJson(Json.toJson(insertImage).as[JsObject])
96101

97102
def runUpsertIntoIndex(indexAlias: String, maybeEsInfo: Option[JsObject]) = {
98103
val esInfo = maybeEsInfo.getOrElse(JsObject.empty)
99104

100105
// On update, we do not want to take the one we have been given unless it is newer - see updateLastModifiedScript script
101106
val updateImage = image.copy(lastModified = None)
102-
val upsertImageAsJson = Json.toJson(updateImage)
107+
val upsertImageAsJson = prepareImageJson(Json.toJson(updateImage).as[JsObject])
103108

104109
val painlessSource =
105110
// If there are old identifiers, then merge any new identifiers into old and use the merged results as the new identifiers
@@ -214,18 +219,24 @@ class ElasticSearch(
214219
def updateEmbedding(id: String, embedding: Embedding, lastModified: DateTime)
215220
(implicit ex: ExecutionContext, logMarker: LogMarker): List[Future[ElasticSearchUpdateResponse]] = {
216221

217-
val replaceEmbeddingScript = "ctx._source.embedding = params.embedding;"
222+
if (!includeDenseVectorMappings) {
223+
logger.info(s"Skipping embedding update for image $id: dense vector mappings are disabled")
224+
List(Future.successful(ElasticSearchUpdateResponse()))
225+
} else {
218226

219-
val scriptSource = loadPainless(replaceEmbeddingScript)
227+
val replaceEmbeddingScript = "ctx._source.embedding = params.embedding;"
220228

221-
val embeddingParameter = asNestedMap(Json.toJson(embedding))
229+
val scriptSource = loadPainless(replaceEmbeddingScript)
222230

223-
val eventualUpdateResponse = migrationAwareUpdater(
224-
requestFromIndexName = indexName => prepareUpdateRequest(indexName, id, scriptSource, lastModified, ("embedding", embeddingParameter)),
225-
logMessageFromIndexName = indexName => s"ES6 updating embedding on image $id in index $indexName"
226-
).incrementOnFailure(metrics.map(_.failedEmbeddingUpdates)) { case _ => true }
231+
val embeddingParameter = asNestedMap(Json.toJson(embedding))
227232

228-
List(eventualUpdateResponse.map(_ => ElasticSearchUpdateResponse()))
233+
val eventualUpdateResponse = migrationAwareUpdater(
234+
requestFromIndexName = indexName => prepareUpdateRequest(indexName, id, scriptSource, lastModified, ("embedding", embeddingParameter)),
235+
logMessageFromIndexName = indexName => s"ES6 updating embedding on image $id in index $indexName"
236+
).incrementOnFailure(metrics.map(_.failedEmbeddingUpdates)) { case _ => true }
237+
238+
List(eventualUpdateResponse.map(_ => ElasticSearchUpdateResponse()))
239+
}
229240
}
230241

231242
def applyImageMetadataOverride(id: String, metadata: Edits, lastModified: DateTime)

0 commit comments

Comments
 (0)