Skip to content

Commit

Permalink
Remove functionality of legacy DB mode (#3539)
Browse files Browse the repository at this point in the history 
Summary:
TensorBoard typically reads from a logdir on disk, but the `--db_import`
and `--db` flags offered an experimental SQLite-backed read path. This
DB mode has always been clearly marked as experimental, and does not
work as advertised. For example, runs were broken in all dashboards.
A data set with *n* runs at top level would show as *n* runs in
TensorBoard, but all of them would have the name `.` and the same color,
and only one of them would actually render data. The images dashboard
was also completely broken and did not show images. And lots of plugins
just didn’t support DB mode at all. Furthermore, the existence of DB
mode is an active impediment to maintenance, and also accounts for some
of the few remaining test cases that require TensorFlow 1.x to run.

This patch removes support for DB mode from all plugins, removes the
`db_connection_provider` field from `TBContext`, and renders the `--db`
and `--db_import` flags functionally inoperative. The plumbing itself
will be removed in a separate commit; a modicum of care is required for
the `TensorBoardInfo`  struct.

Test Plan:
Running with a standard demo logdir, TensorBoard still works fine with
both `--generic_data false` and `--generic_data true`.

wchargin-branch: nodb-functionality
  • Loading branch information
@wchargin @caisq
wchargin authored and caisq committed May 27, 2020
1 parent 4aa6e07 commit 628b78f
Show file tree
Hide file tree
Showing 19 changed files with 67 additions and 2,057 deletions.
8 changes: 0 additions & 8 deletions tensorboard/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,14 +348,6 @@ py_library(
visibility = ["//visibility:public"],
)

py_library(
name = "expect_sqlite3_installed",
# This is a dummy rule used as a sqlite3 dependency in open-source.
# We expect sqlite3 to already be present, as it is part of the standard
# library.
visibility = ["//visibility:public"],
)

py_library(
name = "expect_tensorflow_installed",
# This is a dummy rule used as a TensorFlow dependency in open-source.
Expand Down
2 changes: 0 additions & 2 deletions tensorboard/backend/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,8 @@ py_library(
":path_prefix",
":security_validator",
"//tensorboard:errors",
"//tensorboard:expect_sqlite3_installed",
"//tensorboard:plugin_util",
"//tensorboard/backend/event_processing:data_provider",
"//tensorboard/backend/event_processing:db_import_multiplexer",
"//tensorboard/backend/event_processing:event_accumulator",
"//tensorboard/backend/event_processing:event_multiplexer",
"//tensorboard/plugins/core:core_plugin",
Expand Down
129 changes: 12 additions & 117 deletions tensorboard/backend/application.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import os
import re
import shutil
import sqlite3
import tempfile
import textwrap
import threading
Expand All @@ -51,7 +50,6 @@
from tensorboard.backend import http_util
from tensorboard.backend import path_prefix
from tensorboard.backend import security_validator
from tensorboard.backend.event_processing import db_import_multiplexer
from tensorboard.backend.event_processing import (
data_provider as event_data_provider,
)
Expand Down Expand Up @@ -134,40 +132,18 @@ def standard_tensorboard_wsgi(flags, plugin_loaders, assets_zip_provider):
data_provider = None
multiplexer = None
reload_interval = flags.reload_interval
if flags.db_import:
# DB import mode.
db_uri = flags.db
# Create a temporary DB file if we weren't given one.
if not db_uri:
tmpdir = tempfile.mkdtemp(prefix="tbimport")
atexit.register(shutil.rmtree, tmpdir)
db_uri = "sqlite:%s/tmp.sqlite" % tmpdir
db_connection_provider = create_sqlite_connection_provider(db_uri)
logger.info("Importing logdir into DB at %s", db_uri)
multiplexer = db_import_multiplexer.DbImportMultiplexer(
db_uri=db_uri,
db_connection_provider=db_connection_provider,
purge_orphaned_data=flags.purge_orphaned_data,
max_reload_threads=flags.max_reload_threads,
)
elif flags.db:
# DB read-only mode, never load event logs.
reload_interval = -1
db_connection_provider = create_sqlite_connection_provider(flags.db)
multiplexer = _DbModeMultiplexer(flags.db, db_connection_provider)
else:
# Regular logdir loading mode.
sampling_hints = _parse_samples_per_plugin(flags)
multiplexer = event_multiplexer.EventMultiplexer(
size_guidance=DEFAULT_SIZE_GUIDANCE,
tensor_size_guidance=_apply_tensor_size_guidance(sampling_hints),
purge_orphaned_data=flags.purge_orphaned_data,
max_reload_threads=flags.max_reload_threads,
event_file_active_filter=_get_event_file_active_filter(flags),
)
data_provider = event_data_provider.MultiplexerDataProvider(
multiplexer, flags.logdir or flags.logdir_spec
)
# Regular logdir loading mode.
sampling_hints = _parse_samples_per_plugin(flags)
multiplexer = event_multiplexer.EventMultiplexer(
size_guidance=DEFAULT_SIZE_GUIDANCE,
tensor_size_guidance=_apply_tensor_size_guidance(sampling_hints),
purge_orphaned_data=flags.purge_orphaned_data,
max_reload_threads=flags.max_reload_threads,
event_file_active_filter=_get_event_file_active_filter(flags),
)
data_provider = event_data_provider.MultiplexerDataProvider(
multiplexer, flags.logdir or flags.logdir_spec
)

if reload_interval >= 0:
# We either reload the multiplexer once when TensorBoard starts up, or we
Expand Down Expand Up @@ -226,19 +202,9 @@ def TensorBoardWSGIApp(
:type plugins: list[base_plugin.TBLoader]
"""
db_uri = None
db_connection_provider = None
if isinstance(
deprecated_multiplexer,
(db_import_multiplexer.DbImportMultiplexer, _DbModeMultiplexer),
):
db_uri = deprecated_multiplexer.db_uri
db_connection_provider = deprecated_multiplexer.db_connection_provider
plugin_name_to_instance = {}
context = base_plugin.TBContext(
data_provider=data_provider,
db_connection_provider=db_connection_provider,
db_uri=db_uri,
flags=flags,
logdir=flags.logdir,
multiplexer=deprecated_multiplexer,
Expand Down Expand Up @@ -759,39 +725,6 @@ def _reload():
raise ValueError("unrecognized reload_task: %s" % reload_task)


def create_sqlite_connection_provider(db_uri):
"""Returns function that returns SQLite Connection objects.
Args:
db_uri: A string URI expressing the DB file, e.g. "sqlite:~/tb.db".
Returns:
A function that returns a new PEP-249 DB Connection, which must be closed,
each time it is called.
Raises:
ValueError: If db_uri is not a valid sqlite file URI.
"""
uri = urlparse.urlparse(db_uri)
if uri.scheme != "sqlite":
raise ValueError("Only sqlite DB URIs are supported: " + db_uri)
if uri.netloc:
raise ValueError("Can not connect to SQLite over network: " + db_uri)
if uri.path == ":memory:":
raise ValueError("Memory mode SQLite not supported: " + db_uri)
path = os.path.expanduser(uri.path)
params = _get_connect_params(uri.query)
# TODO(@jart): Add thread-local pooling.
return lambda: sqlite3.connect(path, **params)


def _get_connect_params(query):
params = urlparse.parse_qs(query)
if any(len(v) > 2 for v in params.values()):
raise ValueError("DB URI params list has duplicate keys: " + query)
return {k: json.loads(v[0]) for k, v in params.items()}


def _clean_path(path):
"""Removes a trailing slash from a non-root path.
Expand Down Expand Up @@ -823,44 +756,6 @@ def _get_event_file_active_filter(flags):
return lambda timestamp: timestamp + inactive_secs >= time.time()


class _DbModeMultiplexer(event_multiplexer.EventMultiplexer):
"""Shim EventMultiplexer to use when in read-only DB mode.
In read-only DB mode, the EventMultiplexer is nonfunctional - there is no
logdir to reload, and the data is all exposed via SQL. This class represents
the do-nothing EventMultiplexer for that purpose, which serves only as a
conduit for DB-related parameters.
The load APIs raise exceptions if called, and the read APIs always
return empty results.
"""

def __init__(self, db_uri, db_connection_provider):
"""Constructor for `_DbModeMultiplexer`.
Args:
db_uri: A URI to the database file in use.
db_connection_provider: Provider function for creating a DB connection.
"""
logger.info("_DbModeMultiplexer initializing for %s", db_uri)
super(_DbModeMultiplexer, self).__init__()
self.db_uri = db_uri
self.db_connection_provider = db_connection_provider
logger.info("_DbModeMultiplexer done initializing")

def AddRun(self, path, name=None):
"""Unsupported."""
raise NotImplementedError()

def AddRunsFromDirectory(self, path, name=None):
"""Unsupported."""
raise NotImplementedError()

def Reload(self):
"""Unsupported."""
raise NotImplementedError()


def make_plugin_loader(plugin_spec):
"""Returns a plugin loader for the given plugin.
Expand Down
85 changes: 0 additions & 85 deletions tensorboard/backend/application_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ def __init__(
samples_per_plugin="",
max_reload_threads=1,
reload_task="auto",
db="",
db_import=False,
window_title="",
path_prefix="",
reload_multifile=False,
Expand All @@ -76,8 +74,6 @@ def __init__(
self.samples_per_plugin = samples_per_plugin
self.max_reload_threads = max_reload_threads
self.reload_task = reload_task
self.db = db
self.db_import = db_import
self.window_title = window_title
self.path_prefix = path_prefix
self.reload_multifile = reload_multifile
Expand Down Expand Up @@ -1018,86 +1014,5 @@ def testEmptyWildcardRouteWithSlash(self):
self._test_route("/data/plugin/bar/wildcard/", 404)


class DbTest(tb_test.TestCase):
def testSqliteDb(self):
db_uri = "sqlite:" + os.path.join(self.get_temp_dir(), "db")
db_connection_provider = application.create_sqlite_connection_provider(
db_uri
)
with contextlib.closing(db_connection_provider()) as conn:
with conn:
with contextlib.closing(conn.cursor()) as c:
c.execute("create table peeps (name text)")
c.execute(
"insert into peeps (name) values (?)", ("justine",)
)
db_connection_provider = application.create_sqlite_connection_provider(
db_uri
)
with contextlib.closing(db_connection_provider()) as conn:
with contextlib.closing(conn.cursor()) as c:
c.execute("select name from peeps")
self.assertEqual(("justine",), c.fetchone())

def testTransactionRollback(self):
db_uri = "sqlite:" + os.path.join(self.get_temp_dir(), "db")
db_connection_provider = application.create_sqlite_connection_provider(
db_uri
)
with contextlib.closing(db_connection_provider()) as conn:
with conn:
with contextlib.closing(conn.cursor()) as c:
c.execute("create table peeps (name text)")
try:
with conn:
with contextlib.closing(conn.cursor()) as c:
c.execute(
"insert into peeps (name) values (?)", ("justine",)
)
raise IOError("hi")
except IOError:
pass
with contextlib.closing(conn.cursor()) as c:
c.execute("select name from peeps")
self.assertIsNone(c.fetchone())

def testTransactionRollback_doesntDoAnythingIfIsolationLevelIsNone(self):
# NOTE: This is a terrible idea. Don't do this.
db_uri = (
"sqlite:"
+ os.path.join(self.get_temp_dir(), "db")
+ "?isolation_level=null"
)
db_connection_provider = application.create_sqlite_connection_provider(
db_uri
)
with contextlib.closing(db_connection_provider()) as conn:
with conn:
with contextlib.closing(conn.cursor()) as c:
c.execute("create table peeps (name text)")
try:
with conn:
with contextlib.closing(conn.cursor()) as c:
c.execute(
"insert into peeps (name) values (?)", ("justine",)
)
raise IOError("hi")
except IOError:
pass
with contextlib.closing(conn.cursor()) as c:
c.execute("select name from peeps")
self.assertEqual(("justine",), c.fetchone())

def testSqliteUriErrors(self):
with self.assertRaises(ValueError):
application.create_sqlite_connection_provider("lol:cat")
with self.assertRaises(ValueError):
application.create_sqlite_connection_provider("sqlite::memory:")
with self.assertRaises(ValueError):
application.create_sqlite_connection_provider(
"sqlite://foo.example/bar"
)


if __name__ == "__main__":
tb_test.main()
51 changes: 0 additions & 51 deletions tensorboard/backend/event_processing/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,57 +285,6 @@ py_test(
],
)

py_library(
name = "db_import_multiplexer",
srcs = [
"db_import_multiplexer.py",
],
srcs_version = "PY2AND3",
visibility = ["//visibility:public"],
deps = [
":directory_watcher",
":event_file_loader",
":event_multiplexer",
":io_wrapper",
":sqlite_writer",
"//tensorboard:data_compat",
"//tensorboard/compat:tensorflow",
"//tensorboard/compat/proto:protos_all_py_pb2",
"//tensorboard/util:tb_logging",
"@org_pythonhosted_six",
],
)

py_test(
name = "db_import_multiplexer_test",
size = "small",
srcs = ["db_import_multiplexer_test.py"],
srcs_version = "PY2AND3",
deps = [
":db_import_multiplexer",
"//tensorboard:expect_sqlite3_installed",
"//tensorboard:expect_tensorflow_installed",
"//tensorboard/compat/proto:protos_all_py_pb2",
"//tensorboard/util:tensor_util",
"//tensorboard/util:test_util",
],
)

py_library(
name = "sqlite_writer",
srcs = [
"sqlite_writer.py",
],
srcs_version = "PY2AND3",
visibility = ["//visibility:public"],
deps = [
"//tensorboard/compat:tensorflow",
"//tensorboard/util:tb_logging",
"//tensorboard/util:tensor_util",
"@org_pythonhosted_six",
],
)

py_library(
name = "plugin_asset_util",
srcs = ["plugin_asset_util.py"],
Expand Down
Loading

0 comments on commit 628b78f

Please sign in to comment.