New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 851034 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task



Sign in to add a comment

Define best practices for using leveldb databases and enforce chrome database to follow

Project Member Reported by ssid@chromium.org, Jun 8 2018

Issue description

We have recently numerous memory misuse issues on leveldb database.

 Issue 809762  caused by history service database which had much higher write buffer size than necessary for small database. having large write buffer causes the transactions to grow till that size before getting committed.

 Issue 810237  was similar issue caused by repeated update of database causing transaction logs to grow.

Issue 793109 caused a major memory regression in the new suggestions / ntp feature because of which the launch was delayed. Again due to improper use of leveldb options.
Dan ran some experiments here to show how memory usage varies for downloads database:
https://docs.google.com/spreadsheets/d/1C5DEkUwXbIDEvctjFmobHLKvKohaxB5nqSPAalzW1V8/edit?usp=sharing

The UMA on Android (where memory is usually a constraint) for leveldatabase usage on browser shows:
1.6 MB at median, 23MB at 99th percentile. It can grow to 100s of MB in some cases.
On low-end devices,
It is 100Kb at median, and 6MB at 99th percentile.
https://uma.googleplex.com/p/chrome/histograms/?endDate=20180606&dayCount=1&histograms=Memory.Experimental.Browser2.Small.LevelDatabase&fixupData=true&showMax=true&analysis=0.25%200.5%200.75%200.95%200.99%200.995&filters=platform%2Ceq%2CA%2Cis_low_mem%2Ceq%2CTrue%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial

The goal is:
1. The leveldatabase should define best practices for usage. Find a way for clients to easily follow these standards. Maybe a required enum that says things like "SmallDatabaseWithLotOfTransactions" or "LargeDatabaseWithLessTransactions".
2. The options must be enforced on all the databases on Chrome. All the existing databases must be changed to follow these standards.
3. Any new database that is added should use these new rules.
 

Comment 1 by ssid@chromium.org, Jun 8 2018

Cc: costan@google.com
Cc: harringtond@chromium.org
Cc: -costan@google.com pwnall@chromium.org
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2fcc619128196e2bf6b7ecaee201de1e4ee163ca

commit 2fcc619128196e2bf6b7ecaee201de1e4ee163ca
Author: Siddhartha <ssid@chromium.org>
Date: Fri Sep 21 20:39:39 2018

Set the write buffer size of all proto database

Before this CL lot of proto databases used default write buffer size
which is 4MB. Some databases explicitly set write buffer sizes because
of telemetry regressions observed. The rest of them were not hit by
telemetry stories when they were added. After this CL the default buffer
size will be set to 512KB for high end and 128KB for low end devices.
Most databases don't store a lot of data, but do a lot of transactions.
So, this will cause compaction more often.

BUG=851034

Change-Id: Ia0fb28035a8e6e3221ee7dc843924086d062a52b
Reviewed-on: https://chromium-review.googlesource.com/1236286
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593323}
[modify] https://crrev.com/2fcc619128196e2bf6b7ecaee201de1e4ee163ca/components/download/internal/background_service/download_store.cc
[modify] https://crrev.com/2fcc619128196e2bf6b7ecaee201de1e4ee163ca/components/leveldb_proto/proto_database.cc

Did you consider setting reuse_logs=false?
My understanding is that with reuse_logs=true, the database log is read into memory when the database is opened, and remains in memory. 

This seems problematic considering you always (after some level of db use) pay the overhead of reading O(buffersize) when the database is opened, regardless of how much data you will read or write, and regardless of how much data is actually in the database.

With reuse_logs=false, the log is read once, and is processed instead of kept in memory.

Sign in to add a comment