perf: Hard-limit get_count runtime to 1 second

We have limited count to 1000 by default but this can still easily read
entire table if:
- There are filters and filters don't use index and there aren't 1000
  matching rows.

This unknown tail latency by default is not acceptable, so by default
any count query that fails to return results in 2 seconds will be
terminated and user will have to explicitly ask for the count.
This commit is contained in:
Ankush Menat 2025-01-24 23:26:28 +05:30
parent 10b429ffd6
commit 658e6c5e73
2 changed files with 34 additions and 20 deletions

View file

@ -50,31 +50,43 @@ def get_list():
@frappe.whitelist()
@frappe.read_only()
def get_count() -> int:
def get_count() -> int | None:
args = get_form_params()
if is_virtual_doctype(args.doctype):
controller = get_controller(args.doctype)
count = frappe.call(controller.get_count, args=args, **args)
else:
args.distinct = sbool(args.distinct)
distinct = "distinct " if args.distinct else ""
args.limit = cint(args.limit)
fieldname = f"{distinct}`tab{args.doctype}`.name"
args.order_by = None
return frappe.call(controller.get_count, args=args, **args)
if args.limit:
args.fields = [fieldname]
partial_query = execute(**args, run=0)
count = frappe.db.sql(f"""select count(*) from ( {partial_query} ) p""")[0][0]
args.distinct = sbool(args.distinct)
distinct = "distinct " if args.distinct else ""
args.limit = cint(args.limit)
fieldname = f"{distinct}`tab{args.doctype}`.name"
args.order_by = None
if count == args.limit:
frappe.local.response_headers.set(
"Cache-Control", "private,max-age=300,stale-while-revalidate=10800"
)
# args.limit is specified to avoid getting accurate count.
if not args.limit:
args.fields = [f"count({fieldname}) as total_count"]
return execute(**args)[0].get("total_count")
args.fields = [fieldname]
partial_query = execute(**args, run=0)
# Count queries are notoriously unpredictable based on the type of filters used.
# We should not attempt to fetch accurate count for 2 entire minutes! (default timeout)
# Very short timeout is used to here to set an upper bound on damage a bad request can do.
# Users can request accurate count by dropping limit from arguments.
frappe.db.set_execution_timeout(1)
try:
count = frappe.db.sql(f"""select count(*) from ( {partial_query} ) p""")[0][0]
except Exception as e:
if frappe.db.is_statement_timeout(e): # Skip fetching accurate count
count = None
else:
args.fields = [f"count({fieldname}) as total_count"]
count = execute(**args)[0].get("total_count")
raise
if count == args.limit or count is None:
frappe.local.response_headers.set("Cache-Control", "private,max-age=300,stale-while-revalidate=10800")
return count

View file

@ -620,7 +620,7 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList {
let $count = this.get_count_element();
this.get_count_str().then((count) => {
$count.html(`<span>${count}</span>`);
if (this.count_upper_bound && this.count_upper_bound == this.total_count) {
if (this.count_upper_bound) {
$count.attr(
"title",
__(
@ -1017,13 +1017,15 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList {
Boolean(this.count_upper_bound)
)
.then((total_count) => {
this.total_count = total_count || current_count;
this.total_count = total_count;
this.count_without_children =
count_without_children !== current_count ? count_without_children : undefined;
let count_str;
if (this.total_count === this.count_upper_bound) {
count_str = `${format_number(this.total_count - 1, null, 0)}+`;
} else if (this.total_count == null) {
count_str = "??";
} else {
count_str = format_number(this.total_count, null, 0);
}