Opened 7 years ago

Closed 6 years ago

#2460 closed defect (wontfix)

make ConfigurableClientList::getCachedZoneWriter thread safe

Reported by: jinmei Owned by:
Priority: high Milestone: DNS Outstanding Tasks
Component: data source Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 7 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

This is a continuation of #2459.

While the suggested solution in #2459 solves the immediate problem by
a relatively simple patch, it's suboptimal performance-wise because
getCachedZoneWriter() involves expensive DB accesses several times,
some may be even blocking: one is findZone(), and the other is
getIterator(). The latter is especially problematic in that it opens
a new DB connection.

Fortunately, getIterator() itself should be thread safe as long as
DatabaseAccessor::clone() of the underlying DB accessor
implementation is thread safe (which is the case for our SQLite3 version).

So my suggested better (but requiring larger changes) solution is as
follows:

  • extend ConfigurableClientList so the caller can know if a specified zone is "cached" (and no, I don't like to use getDataSources() for this purpose. IMO it discloses too much internal details of the class and restricts future changes of the implementation). One possibility is to extend find() (like extending exact_match_ to a generic flags).
  • revise getCachedZoneWriter() so it only looks for in-memory data sources. findZone() on in-memory data source should be thread safe, so there's no risk of race here. We'll need to extend findInternal() a bit to make this possible.
  • in b10-auth (or any other possible user of getCachedZoneWriter()), the builder thread first checks if it really needs to do reload using the first extension. It protects this operation by the mutex. If it does, the builder calls the revised getCachedZoneWriter() without the protection of mutex.

Subtickets

Change History (6)

comment:1 Changed 7 years ago by shane

  • Milestone New Tasks deleted

comment:2 Changed 7 years ago by jinmei

See https://lists.isc.org/pipermail/bind10-dev/2013-January/004215.html

I believe this task solves the issue I explained in that message.
While the allowable scalability for SQLite3 may be moot, I suspect
the same discussion will apply to other databases because opening a
new DB connection could be time consuming or even blocking operation.

So I'm proposing it for the next sprint.

comment:3 Changed 7 years ago by jinmei

  • Milestone set to Next-Sprint-Proposed
  • Priority changed from medium to high

comment:4 Changed 7 years ago by jinmei

  • Milestone changed from Previous-Sprint-Proposed to Next-Sprint-Proposed

comment:5 Changed 6 years ago by stephen

  • Milestone set to DNS Outstanding Tasks

comment:6 Changed 6 years ago by tomek

  • Resolution set to wontfix
  • Status changed from new to closed

DNS and BIND10 framework is outside of scope for Kea project.
The corresponding code has been removed from Kea git repository.
If you want to follow up on DNS or former BIND10 issues, see
http://bundy-dns.de project.

Closing ticket.

Note: See TracTickets for help on using tickets.