Commit 87d2a876 authored by JINMEI Tatuya's avatar JINMEI Tatuya
Browse files

[master] Merge branch 'trac1068review2'

with fixing conflicts in database_unittest.cc.
parents c86612cd 83a58b81
......@@ -568,8 +568,8 @@ WARN_LOGFILE =
# directories like "/usr/src/myproject". Separate the files or directories
# with spaces.
INPUT = ../src/lib/cc ../src/lib/config \
../src/lib/cryptolink ../src/lib/dns ../src/lib/datasrc \
INPUT = ../src/lib/exceptions ../src/lib/cc \
../src/lib/config ../src/lib/cryptolink ../src/lib/dns ../src/lib/datasrc \
../src/bin/auth ../src/bin/resolver ../src/lib/bench ../src/lib/log \
../src/lib/log/compiler ../src/lib/asiolink/ ../src/lib/nsas \
../src/lib/testutils ../src/lib/cache ../src/lib/server_common/ \
......
......@@ -80,8 +80,8 @@ typedef boost::shared_ptr<ZoneIterator> ZoneIteratorPtr;
/// disruption with a naive copy it's prohibited explicitly. For the expected
/// usage of the client classes the restriction should be acceptable.
///
/// \todo This class is not complete. It needs more factory methods, for
/// accessing the whole zone, updating it, loading it, etc.
/// \todo This class is still not complete. It will need more factory methods,
/// e.g. for (re)loading a zone.
class DataSourceClient : boost::noncopyable {
public:
/// \brief A helper structure to represent the search result of
......@@ -180,6 +180,65 @@ public:
isc_throw(isc::NotImplemented,
"Data source doesn't support iteration");
}
/// Return an updater to make updates to a specific zone.
///
/// The RR class of the zone is the one that the client is expected to
/// handle (see the detailed description of this class).
///
/// If the specified zone is not found via the client, a NULL pointer
/// will be returned; in other words a completely new zone cannot be
/// created using an updater. It must be created beforehand (even if
/// it's an empty placeholder) in a way specific to the underlying data
/// source.
///
/// Conceptually, the updater will trigger a separate transaction for
/// subsequent updates to the zone within the context of the updater
/// (the actual implementation of the "transaction" may vary for the
/// specific underlying data source). Until \c commit() is performed
/// on the updater, the intermediate updates won't affect the results
/// of other methods (and the result of the object's methods created
/// by other factory methods). Likewise, if the updater is destructed
/// without performing \c commit(), the intermediate updates will be
/// effectively canceled and will never affect other methods.
///
/// If the underlying data source allows concurrent updates, this method
/// can be called multiple times while the previously returned updater(s)
/// are still active. In this case each updater triggers a different
/// "transaction". Normally it would be for different zones for such a
/// case as handling multiple incoming AXFR streams concurrently, but
/// this interface does not even prohibit an attempt of getting more than
/// one updater for the same zone, as long as the underlying data source
/// allows such an operation (and any conflict resolution is left to the
/// specific derived class implementation).
///
/// If \c replace is true, any existing RRs of the zone will be
/// deleted on successful completion of updates (after \c commit() on
/// the updater); if it's false, the existing RRs will be
/// intact unless explicitly deleted by \c deleteRRset() on the updater.
///
/// A data source can be "read only" or can prohibit partial updates.
/// In such cases this method will result in an \c isc::NotImplemented
/// exception unconditionally or when \c replace is false).
///
/// \note To avoid throwing the exception accidentally with a lazy
/// implementation, we still keep this method pure virtual without
/// an implementation. All derived classes must explicitly define this
/// method, even if it simply throws the NotImplemented exception.
///
/// \exception NotImplemented The underlying data source does not support
/// updates.
/// \exception DataSourceError Internal error in the underlying data
/// source.
/// \exception std::bad_alloc Resource allocation failure.
///
/// \param name The zone name to be updated
/// \param replace Whether to delete existing RRs before making updates
///
/// \return A pointer to the updater; it will be NULL if the specified
/// zone isn't found.
virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name& name,
bool replace) const = 0;
};
}
}
......
......@@ -12,6 +12,7 @@
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
#include <string>
#include <vector>
#include <datasrc/database.h>
......@@ -21,6 +22,8 @@
#include <exceptions/exceptions.h>
#include <dns/name.h>
#include <dns/rrclass.h>
#include <dns/rrttl.h>
#include <dns/rrset.h>
#include <dns/rdata.h>
#include <dns/rdataclass.h>
......@@ -29,17 +32,18 @@
#include <boost/foreach.hpp>
#include <string>
using namespace isc::dns;
using std::string;
using namespace std;
using boost::shared_ptr;
using namespace isc::dns::rdata;
namespace isc {
namespace datasrc {
DatabaseClient::DatabaseClient(boost::shared_ptr<DatabaseAccessor>
DatabaseClient::DatabaseClient(RRClass rrclass,
boost::shared_ptr<DatabaseAccessor>
accessor) :
accessor_(accessor)
rrclass_(rrclass), accessor_(accessor)
{
if (!accessor_) {
isc_throw(isc::InvalidParameter,
......@@ -604,5 +608,170 @@ DatabaseClient::getIterator(const isc::dns::Name& name) const {
return (ZoneIteratorPtr(new DatabaseIterator(context, RRClass::IN())));
}
//
// Zone updater using some database system as the underlying data source.
//
class DatabaseUpdater : public ZoneUpdater {
public:
DatabaseUpdater(shared_ptr<DatabaseAccessor> accessor, int zone_id,
const Name& zone_name, const RRClass& zone_class) :
committed_(false), accessor_(accessor), zone_id_(zone_id),
db_name_(accessor->getDBName()), zone_name_(zone_name.toText()),
zone_class_(zone_class),
finder_(new DatabaseClient::Finder(accessor_, zone_id_, zone_name))
{
logger.debug(DBG_TRACE_DATA, DATASRC_DATABASE_UPDATER_CREATED)
.arg(zone_name_).arg(zone_class_).arg(db_name_);
}
virtual ~DatabaseUpdater() {
if (!committed_) {
try {
accessor_->rollbackUpdateZone();
logger.info(DATASRC_DATABASE_UPDATER_ROLLBACK)
.arg(zone_name_).arg(zone_class_).arg(db_name_);
} catch (const DataSourceError& e) {
// We generally expect that rollback always succeeds, and
// it should in fact succeed in a way we execute it. But
// as the public API allows rollbackUpdateZone() to fail and
// throw, we should expect it. Obviously we cannot re-throw
// it. The best we can do is to log it as a critical error.
logger.error(DATASRC_DATABASE_UPDATER_ROLLBACKFAIL)
.arg(zone_name_).arg(zone_class_).arg(db_name_)
.arg(e.what());
}
}
logger.debug(DBG_TRACE_DATA, DATASRC_DATABASE_UPDATER_DESTROYED)
.arg(zone_name_).arg(zone_class_).arg(db_name_);
}
virtual ZoneFinder& getFinder() { return (*finder_); }
virtual void addRRset(const RRset& rrset);
virtual void deleteRRset(const RRset& rrset);
virtual void commit();
private:
bool committed_;
shared_ptr<DatabaseAccessor> accessor_;
const int zone_id_;
const string db_name_;
const string zone_name_;
const RRClass zone_class_;
boost::scoped_ptr<DatabaseClient::Finder> finder_;
};
void
DatabaseUpdater::addRRset(const RRset& rrset) {
if (committed_) {
isc_throw(DataSourceError, "Add attempt after commit to zone: "
<< zone_name_ << "/" << zone_class_);
}
if (rrset.getClass() != zone_class_) {
isc_throw(DataSourceError, "An RRset of a different class is being "
<< "added to " << zone_name_ << "/" << zone_class_ << ": "
<< rrset.toText());
}
if (rrset.getRRsig()) {
isc_throw(DataSourceError, "An RRset with RRSIG is being added to "
<< zone_name_ << "/" << zone_class_ << ": "
<< rrset.toText());
}
RdataIteratorPtr it = rrset.getRdataIterator();
if (it->isLast()) {
isc_throw(DataSourceError, "An empty RRset is being added for "
<< rrset.getName() << "/" << zone_class_ << "/"
<< rrset.getType());
}
string columns[DatabaseAccessor::ADD_COLUMN_COUNT]; // initialized with ""
columns[DatabaseAccessor::ADD_NAME] = rrset.getName().toText();
columns[DatabaseAccessor::ADD_REV_NAME] =
rrset.getName().reverse().toText();
columns[DatabaseAccessor::ADD_TTL] = rrset.getTTL().toText();
columns[DatabaseAccessor::ADD_TYPE] = rrset.getType().toText();
for (; !it->isLast(); it->next()) {
if (rrset.getType() == RRType::RRSIG()) {
// XXX: the current interface (based on the current sqlite3
// data source schema) requires a separate "sigtype" column,
// even though it won't be used in a newer implementation.
// We should eventually clean up the schema design and simplify
// the interface, but until then we have to conform to the schema.
const generic::RRSIG& rrsig_rdata =
dynamic_cast<const generic::RRSIG&>(it->getCurrent());
columns[DatabaseAccessor::ADD_SIGTYPE] =
rrsig_rdata.typeCovered().toText();
}
columns[DatabaseAccessor::ADD_RDATA] = it->getCurrent().toText();
accessor_->addRecordToZone(columns);
}
}
void
DatabaseUpdater::deleteRRset(const RRset& rrset) {
if (committed_) {
isc_throw(DataSourceError, "Delete attempt after commit on zone: "
<< zone_name_ << "/" << zone_class_);
}
if (rrset.getClass() != zone_class_) {
isc_throw(DataSourceError, "An RRset of a different class is being "
<< "deleted from " << zone_name_ << "/" << zone_class_
<< ": " << rrset.toText());
}
if (rrset.getRRsig()) {
isc_throw(DataSourceError, "An RRset with RRSIG is being deleted from "
<< zone_name_ << "/" << zone_class_ << ": "
<< rrset.toText());
}
RdataIteratorPtr it = rrset.getRdataIterator();
if (it->isLast()) {
isc_throw(DataSourceError, "An empty RRset is being deleted for "
<< rrset.getName() << "/" << zone_class_ << "/"
<< rrset.getType());
}
string params[DatabaseAccessor::DEL_PARAM_COUNT]; // initialized with ""
params[DatabaseAccessor::DEL_NAME] = rrset.getName().toText();
params[DatabaseAccessor::DEL_TYPE] = rrset.getType().toText();
for (; !it->isLast(); it->next()) {
params[DatabaseAccessor::DEL_RDATA] = it->getCurrent().toText();
accessor_->deleteRecordInZone(params);
}
}
void
DatabaseUpdater::commit() {
if (committed_) {
isc_throw(DataSourceError, "Duplicate commit attempt for "
<< zone_name_ << "/" << zone_class_ << " on "
<< db_name_);
}
accessor_->commitUpdateZone();
committed_ = true; // make sure the destructor won't trigger rollback
// We release the accessor immediately after commit is completed so that
// we don't hold the possible internal resource any longer.
accessor_.reset();
logger.debug(DBG_TRACE_DATA, DATASRC_DATABASE_UPDATER_COMMIT)
.arg(zone_name_).arg(zone_class_).arg(db_name_);
}
// The updater factory
ZoneUpdaterPtr
DatabaseClient::getUpdater(const isc::dns::Name& name, bool replace) const {
shared_ptr<DatabaseAccessor> update_accessor(accessor_->clone());
const std::pair<bool, int> zone(update_accessor->startUpdateZone(
name.toText(), replace));
if (!zone.first) {
return (ZoneUpdaterPtr());
}
return (ZoneUpdaterPtr(new DatabaseUpdater(update_accessor, zone.second,
name, rrclass_)));
}
}
}
......@@ -15,6 +15,14 @@
#ifndef __DATABASE_DATASRC_H
#define __DATABASE_DATASRC_H
#include <string>
#include <boost/scoped_ptr.hpp>
#include <dns/rrclass.h>
#include <dns/rrclass.h>
#include <dns/rrset.h>
#include <datasrc/client.h>
#include <dns/name.h>
......@@ -109,6 +117,7 @@ public:
* classes in polymorphic way.
*/
virtual ~DatabaseAccessor() { }
/**
* \brief Retrieve a zone identifier
*
......@@ -420,6 +429,35 @@ public:
/// to the method or internal database error.
virtual void rollbackUpdateZone() = 0;
/// Clone the accessor with the same configuration.
///
/// Each derived class implementation of this method will create a new
/// accessor of the same derived class with the same configuration
/// (such as the database server address) as that of the caller object
/// and return it.
///
/// Note that other internal states won't be copied to the new accessor
/// even though the name of "clone" may indicate so. For example, even
/// if the calling accessor is in the middle of a update transaction,
/// the new accessor will not start a transaction to trace the same
/// updates.
///
/// The intended use case of cloning is to create a separate context
/// where a specific set of database operations can be performed
/// independently from the original accessor. The updater will use it
/// so that multiple updaters can be created concurrently even if the
/// underlying database system doesn't allow running multiple transactions
/// in a single database connection.
///
/// The underlying database system may not support the functionality
/// that would be needed to implement this method. For example, it
/// may not allow a single thread (or process) to have more than one
/// database connections. In such a case the derived class implementation
/// should throw a \c DataSourceError exception.
///
/// \return A shared pointer to the cloned accessor.
virtual boost::shared_ptr<DatabaseAccessor> clone() = 0;
/**
* \brief Returns a string identifying this dabase backend
*
......@@ -454,16 +492,19 @@ public:
/**
* \brief Constructor
*
* It initializes the client with a database.
* It initializes the client with a database via the given accessor.
*
* \exception isc::InvalidParameter if database is NULL. It might throw
* \exception isc::InvalidParameter if accessor is NULL. It might throw
* standard allocation exception as well, but doesn't throw anything else.
*
* \param database The database to use to get data. As the parameter
* suggests, the client takes ownership of the database and will
* delete it when itself deleted.
* \param rrclass The RR class of the zones that this client will handle.
* \param accessor The accessor to the database to use to get data.
* As the parameter suggests, the client takes ownership of the accessor
* and will delete it when itself deleted.
*/
DatabaseClient(boost::shared_ptr<DatabaseAccessor> database);
DatabaseClient(isc::dns::RRClass rrclass,
boost::shared_ptr<DatabaseAccessor> accessor);
/**
* \brief Corresponding ZoneFinder implementation
*
......@@ -568,6 +609,7 @@ public:
boost::shared_ptr<DatabaseAccessor> accessor_;
const int zone_id_;
const isc::dns::Name origin_;
/**
* \brief Searches database for an RRset
*
......@@ -614,6 +656,7 @@ public:
bool want_ns, const
isc::dns::Name*
construct_name = NULL);
/**
* \brief Checks if something lives below this domain.
*
......@@ -660,7 +703,17 @@ public:
*/
virtual ZoneIteratorPtr getIterator(const isc::dns::Name& name) const;
/// This implementation internally clones the accessor from the one
/// used in the client and starts a separate transaction using the cloned
/// accessor. The returned updater will be able to work separately from
/// the original client.
virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name& name,
bool replace) const;
private:
/// \brief The RR class that this client handles.
const isc::dns::RRClass rrclass_;
/// \brief The accessor to our database.
const boost::shared_ptr<DatabaseAccessor> accessor_;
};
......@@ -668,4 +721,8 @@ private:
}
}
#endif
#endif // __DATABASE_DATASRC_H
// Local Variables:
// mode: c++
// End:
......@@ -590,3 +590,38 @@ data source.
% DATASRC_UNEXPECTED_QUERY_STATE unexpected query state
This indicates a programming error. An internal task of unknown type was
generated.
% DATASRC_DATABASE_UPDATER_CREATED zone updater created for '%1/%2' on %3
Debug information. A zone updater object is created to make updates to
the shown zone on the shown backend database.
% DATASRC_DATABASE_UPDATER_DESTROYED zone updater destroyed for '%1/%2' on %3
Debug information. A zone updater object is destroyed, either successfully
or after failure of, making updates to the shown zone on the shown backend
database.
%DATASRC_DATABASE_UPDATER_ROLLBACK zone updates roll-backed for '%1/%2' on %3
A zone updater is being destroyed without committing the changes.
This would typically mean the update attempt was aborted due to some
error, but may also be a bug of the application that forgets committing
the changes. The intermediate changes made through the updater won't
be applied to the underlying database. The zone name, its class, and
the underlying database name are shown in the log message.
%DATASRC_DATABASE_UPDATER_ROLLBACKFAIL failed to roll back zone updates for '%1/%2' on %3: %4
A zone updater is being destroyed without committing the changes to
the database, and attempts to rollback incomplete updates, but it
unexpectedly fails. The higher level implementation does not expect
it to fail, so this means either a serious operational error in the
underlying data source (such as a system failure of a database) or
software bug in the underlying data source implementation. In either
case if this message is logged the administrator should carefully
examine the underlying data source to see what exactly happens and
whether the data is still valid. The zone name, its class, and the
underlying database name as well as the error message thrown from the
database module are shown in the log message.
% DATASRC_DATABASE_UPDATER_COMMIT updates committed for '%1/%2' on %3
Debug information. A set of updates to a zone has been successfully
committed to the corresponding database backend. The zone name,
its class and the database name are printed.
......@@ -17,6 +17,8 @@
#include <boost/shared_ptr.hpp>
#include <boost/bind.hpp>
#include <exceptions/exceptions.h>
#include <dns/name.h>
#include <dns/rrclass.h>
#include <dns/rrsetlist.h>
......@@ -793,5 +795,9 @@ InMemoryClient::getIterator(const Name& name) const {
return (ZoneIteratorPtr(new MemoryIterator(zone->impl_->domains_, name)));
}
ZoneUpdaterPtr
InMemoryClient::getUpdater(const isc::dns::Name&, bool) const {
isc_throw(isc::NotImplemented, "Update attempt on in memory data source");
}
} // end of namespace datasrc
} // end of namespace dns
......@@ -266,6 +266,17 @@ public:
/// \brief Implementation of the getIterator method
virtual ZoneIteratorPtr getIterator(const isc::dns::Name& name) const;
/// In-memory data source is read-only, so this derived method will
/// result in a NotImplemented exception.
///
/// \note We plan to use a database-based data source as a backend
/// persistent storage for an in-memory data source. When it's
/// implemented we may also want to allow the user of the in-memory client
/// to update via its updater (this may or may not be a good idea and
/// is subject to further discussions).
virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name& name,
bool replace) const;
private:
// TODO: Do we still need the PImpl if nobody should manipulate this class
// directly any more (it should be handled through DataSourceClient)?
......
......@@ -111,8 +111,7 @@ public:
}
void exec() {
if (sqlite3_step(dbparameters_.statements_[stmt_id_]) !=
SQLITE_DONE) {
if (sqlite3_step(dbparameters_.statements_[stmt_id_]) != SQLITE_DONE) {
sqlite3_reset(dbparameters_.statements_[stmt_id_]);
isc_throw(DataSourceError, "failed to " << desc_ << ": " <<
sqlite3_errmsg(dbparameters_.db_));
......@@ -128,6 +127,7 @@ private:
SQLite3Accessor::SQLite3Accessor(const std::string& filename,
const isc::dns::RRClass& rrclass) :
dbparameters_(new SQLite3Parameters),
filename_(filename),
class_(rrclass.toText()),
database_name_("sqlite3_" +
isc::util::Filename(filename).nameAndExtension())
......@@ -137,6 +137,25 @@ SQLite3Accessor::SQLite3Accessor(const std::string& filename,
open(filename);
}
SQLite3Accessor::SQLite3Accessor(const std::string& filename,
const string& rrclass) :
dbparameters_(new SQLite3Parameters),
filename_(filename),
class_(rrclass),
database_name_("sqlite3_" +
isc::util::Filename(filename).nameAndExtension())
{
LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_SQLITE_NEWCONN);
open(filename);
}
boost::shared_ptr<DatabaseAccessor>
SQLite3Accessor::clone() {
return (boost::shared_ptr<DatabaseAccessor>(new SQLite3Accessor(filename_,
class_)));
}
namespace {
// This is a helper class to initialize a Sqlite3 DB safely. An object of
......
......@@ -71,6 +71,17 @@ public:
*/
SQLite3Accessor(const std::string& filename,
const isc::dns::RRClass& rrclass);
/**
* \brief Constructor
*
* Same as the other version, but takes rrclass as a bare string.
* we should obsolete the other version and unify the constructor to
* this version; the SQLite3Accessor is expected to be "dumb" and
* shouldn't care about DNS specific information such as RRClass.
*/
SQLite3Accessor(const std::string& filename, const std::string& rrclass);
/**
* \brief Destructor
*
......@@ -78,6 +89,10 @@ public:
*/
~SQLite3Accessor();
/// This implementation internally opens a new sqlite3 database for the
/// same file name specified in the constructor of the original accessor.
virtual boost::shared_ptr<DatabaseAccessor> clone();
/**
* \brief Look up a zone
*
......@@ -158,6 +173,8 @@ public:
private:
/// \brief Private database data
boost::scoped_ptr<SQLite3Parameters> dbparameters_;
/// \brief The filename of the DB (necessary for clone())
const std::string filename_;
/// \brief The class for which the queries are done
const std::string class_;
/// \brief Opens the database
......
......@@ -32,6 +32,9 @@ public:
virtual FindResult findZone(const isc::dns::Name&) const {
return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
}
virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name&, bool) const {
return (ZoneUpdaterPtr());
}
};
class ClientTest : public ::testing::Test {
......
This diff is collapsed.
......@@ -197,6 +197,11 @@ TEST_F(InMemoryClientTest, getZoneCount) {
EXPECT_EQ(2, memory_client.getZoneCount());
}
TEST_F(InMemoryClientTest, startUpdateZone) {
EXPECT_THROW(memory_client.getUpdater(Name("example.org"), false),
isc::NotImplemented);
}
// A helper callback of masterLoad() used in InMemoryZoneFinderTest.
void
setRRset(RRsetPtr rrset, vector<RRsetPtr*>::iterator& it) {
......@@ -1097,5 +1102,4 @@ TEST_F(InMemoryZoneFinderTest, getFileName) {
EXPECT_EQ(TEST_DATA_DIR "/root.zone", zone_finder_.getFileName());
EXPECT_TRUE(rootzone.getFileName().empty());
}
}
......@@ -409,6 +409,34 @@ TEST_F(SQLite3Create, lockedtest) {
SQLite3Accessor accessor3(SQLITE_NEW_DBFILE, RRClass::IN());
}