Skip to content

Commit

Permalink
Merge "Use config-tracker db instead of walking the entire graph for …
Browse files Browse the repository at this point in the history 
…CleanupInterest" into R2.20
  • Loading branch information
@opencontrail-ci-admin
Zuul authored and opencontrail-ci-admin committed Jul 23, 2015
2 parents 030584f + 7e358ef commit c7e3500
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 46 deletions.
18 changes: 16 additions & 2 deletions src/ifmap/ifmap_exporter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ IFMapNodeState *IFMapExporter::NodeStateLocate(IFMapNode *node){
IFMapNodeState *state = static_cast<IFMapNodeState *>(
node->GetState(node->table(), tinfo->id()));
if (state == NULL) {
state = new IFMapNodeState();
state = new IFMapNodeState(node);
node->SetState(node->table(), tinfo->id(), state);
}
return state;
Expand Down Expand Up @@ -364,7 +364,7 @@ void IFMapExporter::NodeTableExport(DBTablePartBase *partition,

if (IsFeasible(node)) {
if (state == NULL) {
state = new IFMapNodeState();
state = new IFMapNodeState(node);
entry->SetState(table, tinfo->id(), state);
}
state->SetValid(node);
Expand Down Expand Up @@ -682,6 +682,20 @@ size_t IFMapExporter::ClientConfigTrackerSize(int index) {
return set->size();
}

IFMapExporter::Cs_citer IFMapExporter::ClientConfigTrackerBegin(int index)
const {
ConfigSet *set = client_config_tracker_.at(index);
assert(set);
return set->begin();
}

IFMapExporter::Cs_citer IFMapExporter::ClientConfigTrackerEnd(int index)
const {
ConfigSet *set = client_config_tracker_.at(index);
assert(set);
return set->end();
}

void IFMapExporter::StateInterestSet(IFMapState *state,
const BitSet& interest_bits) {
// Add the node to the config-tracker of all clients that just became
Expand Down
3 changes: 3 additions & 0 deletions src/ifmap/ifmap_exporter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class IFMapExporter {
public:
typedef boost::unordered_set<IFMapState *> ConfigSet;
typedef ConfigSet::size_type CsSz_t;
typedef ConfigSet::const_iterator Cs_citer;
typedef std::vector<ConfigSet *> ClientConfigTracker;
typedef boost::crc_32_type::value_type crc32type;
explicit IFMapExporter(IFMapServer *server);
Expand Down Expand Up @@ -74,6 +75,8 @@ class IFMapExporter {
bool ClientConfigTrackerHasState(int index, IFMapState *state);
bool ClientConfigTrackerEmpty(int index);
size_t ClientConfigTrackerSize(int index);
Cs_citer ClientConfigTrackerBegin(int index) const;
Cs_citer ClientConfigTrackerEnd(int index) const;

void StateInterestSet(IFMapState *state, const BitSet& interest_bits);
void StateInterestOr(IFMapState *state, const BitSet& interest_bits);
Expand Down
35 changes: 24 additions & 11 deletions src/ifmap/ifmap_graph_walker.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,13 +199,8 @@ void IFMapGraphWalker::ResetLinkDeleteClients(const BitSet &bset) {
link_delete_clients_.Reset(bset);
}

void IFMapGraphWalker::CleanupInterest(DBGraphVertex *vertex) {
void IFMapGraphWalker::CleanupInterest(IFMapNode *node, IFMapNodeState *state) {
// interest = interest - rm_mask_ + nmask
IFMapNode *node = static_cast<IFMapNode *>(vertex);
IFMapNodeState *state = exporter_->NodeStateLookup(node);
if (state == NULL) {
return;
}

if (!state->interest().empty() && !state->nmask().empty()) {
IFMAP_DEBUG(CleanupInterest, node->ToString(),
Expand All @@ -232,13 +227,31 @@ void IFMapGraphWalker::CleanupInterest(DBGraphVertex *vertex) {
}

// Cleanup all graph nodes that have a bit set in the remove mask (rm_mask_) but
// were not visited by the walker because there were not reachable after the
// were not visited by the walker because they were not reachable after the
// link delete.
void IFMapGraphWalker::LinkDeleteWalkBatchEnd() {
for (DBGraph::vertex_iterator iter = graph_->vertex_list_begin();
iter != graph_->vertex_list_end(); ++iter) {
DBGraphVertex *vertex = iter.operator->();
CleanupInterest(vertex);
IFMapState *state = NULL;
IFMapNode *node = NULL;
IFMapExporter::Cs_citer iter, end_iter;

for (size_t i = rm_mask_.find_first(); i != BitSet::npos;
i = rm_mask_.find_next(i)) {
iter = exporter_->ClientConfigTrackerBegin(i);
end_iter = exporter_->ClientConfigTrackerEnd(i);
while (iter != end_iter) {
state = *iter;
// Get the iterator to the next element before calling
// CleanupInterest() since the state might be removed from the
// client's config-tracker, thereby invalidating the iterator.
++iter;
if (state->IsNode()) {
node = state->GetIFMapNode();
assert(node);
IFMapNodeState *nstate = exporter_->NodeStateLookup(node);
assert(state == nstate);
CleanupInterest(node, nstate);
}
}
}
rm_mask_.clear();
}
Expand Down
3 changes: 2 additions & 1 deletion src/ifmap/ifmap_graph_walker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class DBGraphEdge;
class DBGraphVertex;
class IFMapExporter;
class IFMapNode;
class IFMapNodeState;
class TaskTrigger;
struct IFMapTypenameFilter;
struct IFMapTypenameWhiteList;
Expand Down Expand Up @@ -40,7 +41,7 @@ class IFMapGraphWalker {
void ProcessLinkAdd(IFMapNode *lnode, IFMapNode *rnode, const BitSet &bset);
void JoinVertex(DBGraphVertex *vertex, const BitSet &bset);
void RecomputeInterest(DBGraphVertex *vertex, int bit);
void CleanupInterest(DBGraphVertex *vertex);
void CleanupInterest(IFMapNode *node, IFMapNodeState *state);
void AddNodesToWhitelist();
void AddLinksToWhitelist();
bool LinkDeleteWalk();
Expand Down
28 changes: 25 additions & 3 deletions src/ifmap/ifmap_update.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,12 @@ IFMapMarker::IFMapMarker()
: IFMapListEntry(MARKER) {
}

IFMapState::IFMapState() : sig_(kInvalidSig), crc_(0) {
IFMapState::IFMapState(IFMapNode *node)
: sig_(kInvalidSig), data_(node), crc_(0) {
}

IFMapState::IFMapState(IFMapLink *link)
: sig_(kInvalidSig), data_(link), crc_(0) {
}

IFMapState::~IFMapState() {
Expand All @@ -90,15 +95,32 @@ void IFMapState::Remove(IFMapUpdate *update) {
update_list_.erase(update_list_.s_iterator_to(*update));
}

IFMapNodeState::IFMapNodeState() {
IFMapNode *IFMapState::GetIFMapNode() const {
if (data_.IsNode()) {
return data_.u.node;
} else {
return NULL;
}
}

IFMapLink *IFMapState::GetIFMapLink() const {
if (data_.IsLink()) {
return data_.u.link;
} else {
return NULL;
}
}

IFMapNodeState::IFMapNodeState(IFMapNode *node)
: IFMapState(node) {
}

bool IFMapNodeState::HasDependents() const {
return !dependents_.empty();
}

IFMapLinkState::IFMapLinkState(IFMapLink *link)
: left_(link), right_(link) {
: IFMapState(link), left_(link), right_(link) {
}

void IFMapLinkState::SetDependency(IFMapNodeState *first,
Expand Down
14 changes: 9 additions & 5 deletions src/ifmap/ifmap_update.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ class IFMapState : public DBState {
> MemberHook;
typedef boost::intrusive::slist<IFMapUpdate, MemberHook> UpdateList;

IFMapState();
IFMapState(IFMapNode *node);
IFMapState(IFMapLink *link);
virtual ~IFMapState();

const BitSet &interest() const { return interest_; }
Expand Down Expand Up @@ -135,20 +136,23 @@ class IFMapState : public DBState {
const crc32type &crc() const { return crc_; }
void SetCrc(crc32type &crc) { crc_ = crc; }
virtual bool CanDelete() = 0;
const IFMapObjectPtr &data() const { return data_; }
IFMapNode *GetIFMapNode() const;
IFMapLink *GetIFMapLink() const;
bool IsNode() const { return data_.IsNode(); }
bool IsLink() const { return data_.IsLink(); }

protected:
static const uint32_t kInvalidSig = -1;
uint32_t sig_;
IFMapObjectPtr data_;

private:

// The set of clients known to be interested in this update.
BitSet interest_;
// The set of clients to which this update has been advertised.
BitSet advertised_;

UpdateList update_list_;

crc32type crc_;
};

Expand All @@ -157,7 +161,7 @@ class IFMapNodeState : public IFMapState {
typedef DependencyList<IFMapLink, IFMapNodeState>::iterator iterator;
typedef DependencyList<IFMapLink, IFMapNodeState>::const_iterator
const_iterator;
IFMapNodeState();
explicit IFMapNodeState(IFMapNode *node);

void SetValid() { IFMapState::SetValid(); }
void SetValid(const IFMapNode *node) { sig_ = 0; }
Expand Down
53 changes: 29 additions & 24 deletions src/ifmap/test/ifmap_exporter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,33 +259,38 @@ TEST_F(IFMapExporterTest, InterestChangeIntersect) {
IFMapMsgLink("domain", "project", "user1", "vnc");
IFMapMsgLink("project", "virtual-network", "vnc", "blue");
IFMapMsgLink("project", "virtual-network", "vnc", "red");
IFMapMsgLink("virtual-machine", "virtual-machine-interface",
"vm_x", "vm_x:veth0");
IFMapMsgLink("virtual-machine-interface", "virtual-network",
"vm_x:veth0", "blue");
IFMapMsgLink("virtual-machine", "virtual-machine-interface",
"vm_w", "vm_w:veth0");
IFMapMsgLink("virtual-machine-interface", "virtual-network",
"vm_w:veth0", "red");
IFMapMsgLink("virtual-machine", "virtual-machine-interface",
"vm_y", "vm_y:veth0");
IFMapMsgLink("virtual-machine-interface", "virtual-network",
"vm_y:veth0", "blue");
IFMapMsgLink("virtual-machine", "virtual-machine-interface",
"vm_z", "vm_z:veth0");
IFMapMsgLink("virtual-machine-interface", "virtual-network",
"vm_z:veth0", "red");
// c1 in blue.
IFMapMsgLink("virtual-machine", "virtual-machine-interface",
"vm_c1", "vm_c1:veth0");
IFMapMsgLink("virtual-machine-interface", "virtual-network",
"vm_c1:veth0", "blue");
// c2 in red.
IFMapMsgLink("virtual-machine", "virtual-machine-interface",
"vm_c2", "vm_c2:veth0");
IFMapMsgLink("virtual-machine-interface", "virtual-network",
"vm_c2:veth0", "red");
// c3 in blue.
IFMapMsgLink("virtual-machine", "virtual-machine-interface",
"vm_c3", "vm_c3:veth0");
IFMapMsgLink("virtual-machine-interface", "virtual-network",
"vm_c3:veth0", "blue");
// c4 in red.
IFMapMsgLink("virtual-machine", "virtual-machine-interface",
"vm_c4", "vm_c4:veth0");
IFMapMsgLink("virtual-machine-interface", "virtual-network",
"vm_c4:veth0", "red");

IFMapMsgLink("virtual-router", "virtual-machine", "192.168.1.1", "vm_x");
IFMapMsgLink("virtual-router", "virtual-machine", "192.168.1.2", "vm_w");
IFMapMsgLink("virtual-router", "virtual-machine", "192.168.1.3", "vm_y");
IFMapMsgLink("virtual-router", "virtual-machine", "192.168.1.1", "vm_c1");
IFMapMsgLink("virtual-router", "virtual-machine", "192.168.1.2", "vm_c2");
IFMapMsgLink("virtual-router", "virtual-machine", "192.168.1.3", "vm_c3");
task_util::WaitForIdle();

IFMapNode *blue = TableLookup("virtual-network", "blue");
ASSERT_TRUE(blue != NULL);
IFMapNodeState *state = exporter_->NodeStateLookup(blue);
ASSERT_TRUE(state != NULL);

// c1 and c3 should get 'blue'.
IFMapUpdate *update = state->GetUpdate(IFMapListEntry::UPDATE);
ASSERT_TRUE(update != NULL);
TASK_UTIL_EXPECT_TRUE(update->advertise().test(c1.index()));
Expand All @@ -294,9 +299,9 @@ TEST_F(IFMapExporterTest, InterestChangeIntersect) {
// Call ProcessQueue() since our QueueActive() does not do anything
ProcessQueue();

IFMapMsgUnlink("virtual-router", "virtual-machine", "192.168.1.2", "vm_w");
IFMapMsgUnlink("virtual-router", "virtual-machine", "192.168.1.3", "vm_y");
IFMapMsgLink("virtual-router", "virtual-machine", "192.168.1.4", "vm_z");
IFMapMsgUnlink("virtual-router", "virtual-machine", "192.168.1.2", "vm_c2");
IFMapMsgUnlink("virtual-router", "virtual-machine", "192.168.1.3", "vm_c3");
IFMapMsgLink("virtual-router", "virtual-machine", "192.168.1.4", "vm_c4");
task_util::WaitForIdle();

// Check that only c3 will receive a delete for blue.
Expand Down Expand Up @@ -339,9 +344,9 @@ TEST_F(IFMapExporterTest, InterestChangeIntersect) {
ProcessQueue();

IFMapMsgUnlink("virtual-machine-interface", "virtual-network",
"vm_z:veth0", "red");
"vm_c4:veth0", "red");
IFMapMsgLink("virtual-machine-interface", "virtual-network",
"vm_z:veth0", "blue");
"vm_c4:veth0", "blue");
task_util::WaitForIdle();

state = exporter_->NodeStateLookup(blue);
Expand Down

0 comments on commit c7e3500

Please sign in to comment.