Skip to content

Commit

Permalink
fix(query): Generalizaing the column filter check to span multiple te…
Browse files Browse the repository at this point in the history
…nants instead of workspace
  • Loading branch information
sandeep6189 committed Sep 16, 2024
1 parent 4320860 commit 0819c1f
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 12 deletions.
3 changes: 3 additions & 0 deletions core/src/main/resources/filodb-defaults.conf
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,9 @@ filodb {
# If unspecified, all workspaces are disabled by default.
workspaces-disabled-max-min = ["disabled_ws"]

# config to figure out the tenant column filter to enable max min column selection
max-min-tenant-column-filter = "_ws_"

routing {
enable-remote-raw-exports = false
max-time-range-remote-raw-export = 180 minutes
Expand Down
3 changes: 3 additions & 0 deletions core/src/main/scala/filodb.core/GlobalConfig.scala
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,7 @@ object GlobalConfig extends StrictLogging {
case true => Some(systemConfig.getStringList("filodb.query.workspaces-disabled-max-min").asScala.toSet)
}

// Column filter used to check the enabling/disabling the use of max-min columns
val maxMinTenantColumnFilter = systemConfig.getString("filodb.query.max-min-tenant-column-filter")

}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import filodb.core.query.{ColumnFilter, QueryConfig, QueryContext, QuerySession,
import filodb.core.query.Filter.Equals
import filodb.core.store._
import filodb.query.Query.qLogger
import filodb.query.TsCardinalities

final case class UnknownSchemaQueryErr(id: Int) extends
Exception(s"Unknown schema ID $id during query. This likely means a schema config change happened and " +
Expand Down Expand Up @@ -58,14 +57,15 @@ final case class MultiSchemaPartitionsExec(queryContext: QueryContext,
}

/**
* @param ws workspace name
* @return true if the config is defined AND ws is not in the list of disabled workspaces. false otherwise
* @param maxMinTenantValue tenant value string such as workspace, app etc
* @return true if the config is defined AND maxMinTenantValue is not in the list of disabled workspaces.
* false otherwise
*/
def isMaxMinEnabledForWorkspace(ws: Option[String]) : Boolean = {
ws.isDefined match {
def isMaxMinColumnsEnabled(maxMinTenantValue: Option[String]) : Boolean = {
maxMinTenantValue.isDefined match {
// we are making sure that the config is defined to avoid any accidental "turn on" of the feature when not desired
case true => (GlobalConfig.workspacesDisabledForMaxMin.isDefined) &&
(!GlobalConfig.workspacesDisabledForMaxMin.get.contains(ws.get))
(!GlobalConfig.workspacesDisabledForMaxMin.get.contains(maxMinTenantValue.get))
case false => false
}
}
Expand All @@ -77,7 +77,8 @@ final case class MultiSchemaPartitionsExec(queryContext: QueryContext,
Kamon.currentSpan().mark("filtered-partition-scan")
var lookupRes = source.lookupPartitions(dataset, partMethod, chunkMethod, querySession)
val metricName = filters.find(_.column == metricColumn).map(_.filter.valuesStrings.head.toString)
val ws = filters.find(x => x.column == TsCardinalities.LABEL_WORKSPACE && x.filter.isInstanceOf[Equals])
val maxMinTenantFilter = filters
.find(x => x.column == GlobalConfig.maxMinTenantColumnFilter && x.filter.isInstanceOf[Equals])
.map(_.filter.valuesStrings.head.toString)
var newColName = colName

Expand Down Expand Up @@ -116,14 +117,14 @@ final case class MultiSchemaPartitionsExec(queryContext: QueryContext,
// This code is responsible for putting exact IDs needed by any range functions.
val colIDs1 = getColumnIDs(sch, newColName.toSeq, rangeVectorTransformers)

val colIDs = isMaxMinEnabledForWorkspace(ws) match {
val colIDs = isMaxMinColumnsEnabled(maxMinTenantFilter) match {
case true => addIDsForHistMaxMin(sch, colIDs1)
case _ => colIDs1
}

// Modify transformers as needed for histogram w/ max, downsample, other schemas
val newxformers1 = newXFormersForDownsample(sch, rangeVectorTransformers)
val newxformers = isMaxMinEnabledForWorkspace(ws) match {
val newxformers = isMaxMinColumnsEnabled(maxMinTenantFilter) match {
case true => newXFormersForHistMaxMin(sch, colIDs, newxformers1)
case _ => newxformers1
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -458,9 +458,9 @@ class MultiSchemaPartitionsExecSpec extends AnyFunSpec with Matchers with ScalaF
it("test isMaxMinEnabledForWorkspace correctly returns expected values") {
val execPlan = MultiSchemaPartitionsExec(QueryContext(), dummyDispatcher, MMD.histMaxMinDS.ref, 0,
Seq(), AllChunkScan, "_metric_", colName = Some("h"))
execPlan.isMaxMinEnabledForWorkspace(Some("demo")) shouldEqual true
execPlan.isMaxMinEnabledForWorkspace(Some(GlobalConfig.workspacesDisabledForMaxMin.get.head)) shouldEqual false
execPlan.isMaxMinEnabledForWorkspace(None) shouldEqual false
execPlan.isMaxMinColumnsEnabled(Some("demo")) shouldEqual true
execPlan.isMaxMinColumnsEnabled(Some(GlobalConfig.workspacesDisabledForMaxMin.get.head)) shouldEqual false
execPlan.isMaxMinColumnsEnabled(None) shouldEqual false
}

it("should return chunk metadata from MemStore") {
Expand Down

0 comments on commit 0819c1f

Please sign in to comment.