6 years agoClean up to remove controller/opendaylight/commons/checkstyle 89/45589/3
Michael Vorburger [Wed, 14 Sep 2016 15:20:20 +0000 (17:20 +0200)]
Clean up to remove controller/opendaylight/commons/checkstyle

Main reason for finally doing this longer overdue clean-up now is that
it appears that the odlparent Checkstyle rules are not effective (in-IDE
and likely also on CLI, not tested) for controller projects such as
md-sal/binding-broker (where a number of violations should have shown
but didn't); and I suspect this may because of a precedence conflict
with these (old) Checkstyle rules, which now are not needed anymore.

Unless I'm missing something, normally (hopefully) this should have no
remaining impact anywhere outside of controller; I have done a
(relatively quick) grep for all 3 of "commons.checkstyle.version" &
"<artifactId>checkstyle" & "checkstyle-logging.xml" (all of which this
removes) in all of autorelease, and have found a few references in 3 old
projects (lacp, alto, netconf; note that netide, ttp used to have
dependencies to controller's checkstyle, but those project have
meanwhile cleaned that up themselves), which the linked changes
will have removed, so... should now be OK.

Change-Id: If6484abd36b01c49c5da5bf277809fb053da0001
Signed-off-by: Michael Vorburger <>
6 years agoMove sal-remote to sal-rest-connector 76/44576/5
Alexis de Talhouët [Tue, 23 Aug 2016 22:29:28 +0000 (18:29 -0400)]
Move sal-remote to sal-rest-connector

This module is only used by sal-rest-connnector bundle,
moreover, it is intended to create notification stream
and/or register a data change event which is done using
RESTCONF, thus it make sense to move it to the appropriate
I beleive this is a leftover of the migration that happened
when controller was splitted out..

This is the associated patch in Netconf project:

Change-Id: Ied4bb2b4b04d36298ca14f9f7926e4aa52de7be2
Signed-off-by: Alexis de Talhouët <>
6 years agoMechanical code cleanup (sal-akka-raft) 84/46084/3
Stephen Kitt [Thu, 22 Sep 2016 16:21:18 +0000 (18:21 +0200)]
Mechanical code cleanup (sal-akka-raft)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: I02567e9b21a73406863797a5a4eaa257e6eb20f2
Signed-off-by: Stephen Kitt <>
6 years agoUse the diamond operator in generated Java 77/35977/2
Stephen Kitt [Wed, 9 Mar 2016 11:12:44 +0000 (12:12 +0100)]
Use the diamond operator in generated Java

Now that javaparser is used to verify the generated code, use <>.

Change-Id: I9ef5ad62108b91eba8565329386f96c777b03cdf
Signed-off-by: Stephen Kitt <>
6 years agoBug 6540: EOS - Prune pending owner change commits on leader change 16/45516/4
Tom Pantelis [Thu, 8 Sep 2016 14:16:53 +0000 (10:16 -0400)]
Bug 6540: EOS - Prune pending owner change commits on leader change

When the shard leader is isolated, it attempts to re-assign ownership for down
peers. However, since it's isolated, it can't commit the modifications. If the
majority partition elects a new leader, when the partition is healed, the old
leader tries to forward the pending owner change commits to the new leader.
However this is problematic as the criteria used to determine the new owner is
stale and owner changes should only be committed by a valid leader. Since the
old leader is no longer the leader, it should not forward pending owner change
commits. However it still should forward local candidate change commits.

So I modified EntityOwnershipShardCommitCoordinator#onStateChange to iterate
the pending Modifications and remove WRITE modifications for the owner leaf
when the shard has transitioned to having a remote leader.

I also fixed an issue in EntityOwnershipShard#onCandidateRemoved that was
intermittently revealed by unit tests. Say candidate1 and candidate2 are
removed quickly for an entity and candidate1 is the current owner.
onCandidateRemoved is called for candidate1 and commits an update to write
candidate2 as the owner. If the write commit is still pending when
onCandidateRemoved is called for candidate2, the current owner will still
be candidate1 and the "message.getRemovedCandidate().equals(currentOwner)"
check will fail and thus the owner isn't cleared and candidate2 will remain
as owner. This results in a node being the owner w/o being in the candidate
list. (This patch may fix Bug 6672 as well)

A new testLeaderIsolation case was added to EntityOwnershipShardTest. Also I
reworked the tests and removed the use of the MockFollower and MockLeader
actors for consistency and also so the tests use the real EOS shard.

Change-Id: I5039b07d02f8571ee2d1affb0f364ea278641e91
Signed-off-by: Tom Pantelis <>
6 years agoBug 6540: Fix journal issues on leader changes 15/45515/4
Tom Pantelis [Fri, 9 Sep 2016 18:08:03 +0000 (14:08 -0400)]
Bug 6540: Fix journal issues on leader changes

Fixed a couple issues with journal syncing on leader changes and isolation.

Consider the scenario where a leader is isolated and the majority partition elects
a new leader and both sides of the partition attempt to commit entries independently.
Say the term was 1 and last journal index was 2 prior to isolation and was replicated
to all followers and applied to state. After isolation, the isolated leader appends a
new entry with index 3 and attempts to replicate but fails to reach consensus.
Meanwhile, the new leader appends its own new entry with index 3 and is successfully
replicated to the remaining follower and applied to state. The commitIndex in the
majority partition is now 3. The new leader attempts to send AppendEntries to the
isolated leader but doesn't get any replies so it marks it as inactive.

When the partition is healed, the isolated leader converts to follower when it hears
from the new leader with the higher term. Since the new leader has marked the isolated
leader as inactive, the initial AppendEntries that the previous leader sees will have
no entries and the leaderCommitIndex will be 3. This is greater than the current
commitIndex 2 so the previous leader will update its commitIndex to 3 and apply its
entry with index 3 to the state. However this entry was from the previous term 1 which
was not replicated to a majority of the nodes and conflicts with the new leader's entry
with index 3 and term 2. This is a violation of raft.

This violation occurs as a result of the new leader not sending any entries until it
knows the follower is active. This is for efficiency to avoid continuously trying to
send entries when a follower is down. This is fine however the leader should not send
its current commit index either since it doesn't know the state of the follower. The
intention of the empty AppendEntries in this case is to re-establish connectivity with
the follower and thus should not cause any state change in the follower. Therefore I
changed the code to send leaderCommitIndex as -1 if the follower is inactive.

The other case where the leader purposely sends an empty AppendEntries is when the
leader is in the process of installing a snapshot on a follower, as indicated by the
presence of a LeaderInstallSnapshotState instance in the FollowerLogInformation. The
empty AppendEntries is still sent at the heartbeat interval to prevent an election
timeout in case the snapshot capture/transfer is delayed. Again, the AppendEntries
should not cause any state change in the follower so I also changed the leader to send
-1 for the leaderCommitIndex. As a result, I also changed it so that the leader
immeditely records a LeaderInstallSnapshotState instance in the FollowerLogInformation
when it initiates the async snapshot capture. Previously this was done when the capture
completed and the RaftActor sent the SendInstallSnapshot message to the leader
behavior. However it may take some time to capture the snapshot and intervening AppendEntries heart beats may be sent to the follower.

The other issue in the above scenario is that the conflict with entry 3 is not
immediately detected. On the first AppendEntries, the previous leader reports back
a successful reply with lastLogIndex 3 and lastLogTerm 1 b/c the previous
index (2) and term (1) didn't conflict. The new leader sets the previous leader's
match index to 3 and thinks index 3 has been replicated to all the followers and
trims its in-memory log at index 2. Eventually when the next entry with index 4 is
replicated, the previous leader will detect the conflict as the leader's previous
log index 3 and term 2 will be sent in the next AppendEntries. The new leader will
backtrack and eventually install a snapshot to sync the previous leader however
it's inefficient and should be unnecessary. The leader should detect the conflict
immediately on the first AppendEntries reply. So I changed handleAppendEntriesReply
to check that the follower's lastLogTerm matches the leader's term for that index.
If not, the leader sets the follower's next index to lastLogTerm - 1. This prevents
the leader from trimming its log and the next AppendEntries will include the
conflicting entry which the follower will remove/replace.

Change-Id: I7a0282cc4078f33ffd049e4a0eb4feff6230510d
Signed-off-by: Tom Pantelis <>
6 years agoBug 6540: Move LeaderInstallSnapshotState to FollowerLogInformation 14/45514/3
Tom Pantelis [Fri, 9 Sep 2016 21:36:03 +0000 (17:36 -0400)]
Bug 6540: Move LeaderInstallSnapshotState to FollowerLogInformation

AbstractLeader maintains a Map of followerId -> LeaderInstallSnapshotState
in parallel to the Map of followerId -> FollowerLogInformation. It makes
sense to move the LeaderInstallSnapshotState into the FollowerLogInformation
instead of maintaining 2 Maps.

Change-Id: Ia0b58fad9bb2fde42d8c1ba4b0f7aae4eb11abb5
Signed-off-by: Tom Pantelis <>
6 years agoBug 6540: Refactor FollowerToSnapshot to its own class 13/45513/2
Tom Pantelis [Fri, 9 Sep 2016 18:10:47 +0000 (14:10 -0400)]
Bug 6540: Refactor FollowerToSnapshot to its own class

Refactored FollowerToSnapshot to its own class and renamed to
LeaderInstallSnapshotState. This will facilitate subsequent patches.

Change-Id: Ie2540ddce1869a9972c8f3d547b0567c3d663aff
Signed-off-by: Tom Pantelis <>
6 years agoBug 6794 - Deprecate eventbus, async-eventbus and fixed-threadpool 38/46138/6
Alexis de Talhouët [Fri, 23 Sep 2016 15:46:58 +0000 (11:46 -0400)]
Bug 6794 - Deprecate eventbus, async-eventbus and fixed-threadpool

Those modules aren't used anymore, I couldn't find any reference of them,
expect some in netconf regarding the fixed-threadpool. But those
reference are coming from big xml test files that I suppose are pretty odl.

This deprecation notice is to warn potential downstream consumer that those
are now deprecated, and that those will be removed in next release, Nitrogen.

Change-Id: I8810b39a632c2d4c678c31bb46b8284aa33593f2
Signed-off-by: Alexis de Talhouët <>
6 years agoBug 6795 - Deprecated sal-dom-xsql 39/46139/3
Alexis de Talhouët [Fri, 23 Sep 2016 15:50:45 +0000 (11:50 -0400)]
Bug 6795 - Deprecated sal-dom-xsql

This deprecation notice is to warn potential downstream consumer that
those are now deprecated, and that those will be removed in next release,

Change-Id: I1a81f76f3b6bc5a976e0422548a117b7a25bead5
Signed-off-by: Alexis de Talhouët <>
6 years agoMechanical code cleanup (sal-remoterpc-connector) 67/46067/3
Stephen Kitt [Thu, 22 Sep 2016 15:22:45 +0000 (17:22 +0200)]
Mechanical code cleanup (sal-remoterpc-connector)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: I502b5887cadb1a7cdbc42e7b817495f9c767a4a1
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (commons) 65/46065/3
Stephen Kitt [Thu, 22 Sep 2016 15:08:38 +0000 (17:08 +0200)]
Mechanical code cleanup (commons)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: If1805b63e52c1ab0c68983e7856e7ccf184cb2a9
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (sal-binding-api) 83/46083/3
Stephen Kitt [Thu, 22 Sep 2016 16:20:18 +0000 (18:20 +0200)]
Mechanical code cleanup (sal-binding-api)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: Ia2f892083eb30b898fbd544ec62620ee6976a553
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (sal-akka-raft-example) 90/46090/3
Stephen Kitt [Thu, 22 Sep 2016 16:31:07 +0000 (18:31 +0200)]
Mechanical code cleanup (sal-akka-raft-example)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: Ie8f6cd7df4287d427e9ecb8174bcd8c79a80c4de
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (sal-common-impl) 91/46091/3
Stephen Kitt [Thu, 22 Sep 2016 16:32:09 +0000 (18:32 +0200)]
Mechanical code cleanup (sal-common-impl)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: I94484029e3967dfb16343e61432f0945659c14b3
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (sal-common-api) 86/46086/3
Stephen Kitt [Thu, 22 Sep 2016 16:23:57 +0000 (18:23 +0200)]
Mechanical code cleanup (sal-common-api)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: I710395170295960e00d46ceb9540994c9c49c96e
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (sal-connector-api) 87/46087/3
Stephen Kitt [Thu, 22 Sep 2016 16:26:25 +0000 (18:26 +0200)]
Mechanical code cleanup (sal-connector-api)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: I7771fe4533469229b319c5d0590d26922a389f9e
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (sal-distributed-datastore) 70/46070/3
Stephen Kitt [Thu, 22 Sep 2016 15:28:27 +0000 (17:28 +0200)]
Mechanical code cleanup (sal-distributed-datastore)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: I3eb023d9f5b34ce83e69c746efd1ff464bfd6789
Signed-off-by: Stephen Kitt <>
6 years agoCode cleanup (sal-common-util) 74/46074/3
Stephen Kitt [Thu, 22 Sep 2016 15:45:37 +0000 (17:45 +0200)]
Code cleanup (sal-common-util)

* Remove unnecessary type specifiers (use Java 7 <>)
* Inline unnecessary temporary variables

Change-Id: I8600988018fd15a711ba4b63d8b947127fb64d7a
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (sal-dom-api) 85/46085/3
Stephen Kitt [Thu, 22 Sep 2016 16:22:29 +0000 (18:22 +0200)]
Mechanical code cleanup (sal-dom-api)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: I88c96965215d2f5dbf60967afe4a4ddbaed0072f
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (sal-dom-broker) 81/46081/3
Stephen Kitt [Thu, 22 Sep 2016 16:18:12 +0000 (18:18 +0200)]
Mechanical code cleanup (sal-dom-broker)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: I4fa4890915dd1bf80dfb69378a38dd2b6c9d7043
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (sal-dom-spi) 73/46073/3
Stephen Kitt [Thu, 22 Sep 2016 15:35:14 +0000 (17:35 +0200)]
Mechanical code cleanup (sal-dom-spi)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: I68a6f9ee118c8ff47bc2511efa234b93b09de32a
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (sal-dom-xsql) 72/46072/3
Stephen Kitt [Thu, 22 Sep 2016 15:31:11 +0000 (17:31 +0200)]
Mechanical code cleanup (sal-dom-xsql)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: I213dd654c1ba13675ebba3c85d4cb549a86e11b8
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (sal-inmemory-datastore) 71/46071/3
Stephen Kitt [Thu, 22 Sep 2016 15:30:11 +0000 (17:30 +0200)]
Mechanical code cleanup (sal-inmemory-datastore)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: I994a877a310241da4079a242f9196536c13747dd
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (config) 66/46066/3
Stephen Kitt [Thu, 22 Sep 2016 15:12:00 +0000 (17:12 +0200)]
Mechanical code cleanup (config)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: Iefd8363a5eb120fdd43a4632b9e3db0e7e347dba
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (sal-clustering-commons) 68/46068/3
Stephen Kitt [Thu, 22 Sep 2016 15:25:04 +0000 (17:25 +0200)]
Mechanical code cleanup (sal-clustering-commons)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: I37133b6c4737bd33926021056c244789856622c2
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (messagebus-spi) 89/46089/3
Stephen Kitt [Thu, 22 Sep 2016 16:28:29 +0000 (18:28 +0200)]
Mechanical code cleanup (messagebus-spi)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: I75e706701f0bd63fd483079aa59ee4da8b88e0e4
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (cds-access-client) 88/46088/3
Stephen Kitt [Thu, 22 Sep 2016 16:27:29 +0000 (18:27 +0200)]
Mechanical code cleanup (cds-access-client)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: Ia7b0f4eb06f033166e9d959be8c28c7cdc132116
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (blueprint) 64/46064/3
Stephen Kitt [Thu, 22 Sep 2016 15:06:14 +0000 (17:06 +0200)]
Mechanical code cleanup (blueprint)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: Iec9055685d1e4f1bba29f20de6da1cd6e14d5503
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (benchmark) 62/46062/5
Stephen Kitt [Thu, 22 Sep 2016 14:57:35 +0000 (16:57 +0200)]
Mechanical code cleanup (benchmark)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: I8405a58fccb12123bc7f2b5902b85c64ca35887e
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (sal-dom-config) 80/46080/3
Stephen Kitt [Thu, 22 Sep 2016 16:16:51 +0000 (18:16 +0200)]
Mechanical code cleanup (sal-dom-config)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: I8cd5f5323c5b8cf62bc583a6fab14523deeff9dd
Signed-off-by: Stephen Kitt <>
6 years agoMechanical code cleanup (sal-cluster-admin) 69/46069/2
Stephen Kitt [Thu, 22 Sep 2016 15:26:28 +0000 (17:26 +0200)]
Mechanical code cleanup (sal-cluster-admin)

* Remove unnecessary type specifiers (use Java 7 <>)
* Remove unnecessary "extends Object" declarations
* Remove unnecessary semi-colons
* Merge identical catch blocks
* Remove redundant modifiers:
  - enum constructors are private by default
  - interface properties are public static final by default
  - interface methods are public abstract by default
  - interfaces are abstract by default
  - inner interfaces are static by default
  - inner classes in interfaces are public static by default

Change-Id: I18aed08bd6670362b679d42ff45f7eae75647ef4
Signed-off-by: Stephen Kitt <>
6 years agoGenerate interface methods without "public" 80/45980/2
Stephen Kitt [Wed, 21 Sep 2016 14:21:26 +0000 (16:21 +0200)]
Generate interface methods without "public"

Method now tracks visibility explicitly; declarations have a blank
specifier, definitions have "public".

Change-Id: I6b1ce1c58d4f9f861e3117a6bf34aa5c9d6c5aa4
Signed-off-by: Stephen Kitt <>
6 years agoBug 6003: Create config-filtering-parent 27/39427/7
Vratko Polak [Tue, 23 Aug 2016 17:14:09 +0000 (19:14 +0200)]
Bug 6003: Create config-filtering-parent

The Bug requests an addition of resource filtering functionality to config-parent.
There are several issues with replacing config-parent functionality:
- It is a top-level non-karaf feature, subject to API freeze.
- maven-resource-plugin operates on directories (as opposed to files).
- Users (ODL or otherwise) may change config.file value.
+ Groovy script can be used to extract directory name...
- but it is not easy propagate computed values to sibling plugins.
- Using two profiles in config-parent does not work easily
  (unless creation of temporary files in local filesystem is abused).

Therefore, a descendant parent pom is defined in this Change.
Presence of ${config.file} activates profiles in both config-parent and config-filtering-parent.

Small edits to config-plugin:
+ Add profile ID to distinguish filtering and non-filtering profiles.

Change-Id: Ia9ef770be232e04e4ba73f634a51b50665ee18ab
Signed-off-by: Vratko Polak <>
6 years agoAdded the 'number of data changes' parameter to dsbenchmark test start outputs 48/45848/2
Jan [Mon, 19 Sep 2016 22:32:55 +0000 (15:32 -0700)]
Added the 'number of data changes' parameter to dsbenchmark test start outputs

Change-Id: Ibc3826e1c0758ebd6cbf30e664a5ecc6a79ec317
Signed-off-by: Jan <>
6 years agoRemove deprecated RaftActor inner classes 25/36125/4
Tom Pantelis [Mon, 19 Sep 2016 19:19:35 +0000 (15:19 -0400)]
Remove deprecated RaftActor inner classes

These classes have been split out and deprecated in Lithium timeframe,
remove them.

Change-Id: If79245a39202e3bbfc6797d1a570ce8fe5af4363
Signed-off-by: Robert Varga <>
Signed-off-by: Tom Pantelis <>
6 years agoFix a few javadoc errors 52/45152/2
Robert Varga [Mon, 5 Sep 2016 10:34:23 +0000 (12:34 +0200)]
Fix a few javadoc errors

Fixes a reference and a parameter name.

Change-Id: Ibd625211d2a6873766bb1a0cb666445d39a323ed
Signed-off-by: Robert Varga <>
6 years agoBug 6659: Fix intermittent PartitionedCandidateOnStartupElectionScenarioTest failure 53/45353/2
Tom Pantelis [Wed, 7 Sep 2016 20:42:51 +0000 (16:42 -0400)]
Bug 6659: Fix intermittent PartitionedCandidateOnStartupElectionScenarioTest failure

The didn't setup the SimpleReplicatedLog, commitIndex, lastAppliedIndex correctly.

Change-Id: I76fbf98f1a227245ca3a61e399258bd3bd4e743a
Signed-off-by: Tom Pantelis <>
(cherry picked from commit 369664974cc0ed545b86aa78c400bff743b7b178)

6 years agoBug 6540: EOS - Rework behavior of onPeerDown 29/45129/5
Tom Pantelis [Sun, 4 Sep 2016 01:08:41 +0000 (21:08 -0400)]
Bug 6540: EOS - Rework behavior of onPeerDown modified the behavior of
onPeerDown to remove all the down node's candidates. However this behavior is
problematic in the case when the shard leader is isolated. The majority partition
will elect a new leader which temporarily results in split-brain and 2 leaders
which independently attempt to remove the other side's candidates. When the partition
is healed, all hell breaks loose trying to reconcile their differences. This is
compounded with the singleton service because it uses 2 entities that are related
to one another.

To alleviate this, I reverted back to the behavior of selecting a new owner for
the entities owned by the down node and leaving the down node as a candidate.
In the case where the down node is the only candidate, it leaves it as the owner.
This doesn't hurt anything and avoids complications with having to re-instate the
down node as owner when it re-joins if it was actually isolated. The idea here is
to keep its candidacy to minimize disruption until proven otherwise since we don't
know if the downed node's process is actually still alive. If another node registers
a candidate it will replace the down node as the owner.

To handle the case where the down node actually restarted, after startup when it
first hears from the leader, it sends a RemoveAllCandidates message to the leader to
remove it from all entities. This cleans out stale candidates should no local client
register a candidate in the new incarnation.

The unit tests revealed an orthogonal issue with the PreLeader state. The PreLeader
switches to Leader when the commit index is up to date but before applying the entries
to the state. However the EOS may commit modifications immediately before the
ApplyState message for prior entries is received. This can result in the "Store tree X  and candidate base Y differ" exception. So I modified the PreLeader behavior to
switch to Leader when the last applied index is up to date. This makes sense b/c
the PreLeader bevavior is intended to protect the state from inconsistencies.

I also fixed a couple bugs where the downPeerMemberNames was accessed with a String
rather than a MemberName instance. This was a remnant of changing downPeerMemberNames
to store MemberName.

Change-Id: I326660c172353539146a2216cc8a70a4b842affe
Signed-off-by: Tom Pantelis <>
6 years agoDeprecated in-memory datastore 82/44882/3
Alexis de Talhouët [Tue, 30 Aug 2016 18:31:19 +0000 (14:31 -0400)]
Deprecated in-memory datastore

Change-Id: Ib960e71d0e73b0ebc72be3279345425853ca50cb
Signed-off-by: Alexis de Talhouët <>
6 years agoNotify listeners on applySnapshot 28/45028/1
Tom Pantelis [Thu, 1 Sep 2016 16:14:07 +0000 (12:14 -0400)]
Notify listeners on applySnapshot

When a snapshot is applied from the leader, we weren't notifying data change listeners.
We should.

Change-Id: If721c2ce7e6f27aa01f7babc0a0ad3c4468840c1
Signed-off-by: Tom Pantelis <>
6 years agoBug 6587: Retain state when transitioning between Leader and IsolatedLeader 99/44899/4
Tom Pantelis [Wed, 31 Aug 2016 04:55:57 +0000 (00:55 -0400)]
Bug 6587: Retain state when transitioning between Leader and IsolatedLeader

If there's a transaction in the COMMIT_PENDING state, ie it has been persisted and
is in the process of being replicated, and the Leader switches to IsolatedLeader, the
ClientRequestTracker state is lost. As a result when the follower(s) come back and
replication consensus is achieved and the tx is applied to state, the tx ID isn't
available and the ShardDataTree applies it as a foreign candidate, leaving the
tx in the pending queue. This prevents subsequent transactions from making progress.

To fix this, we need to retain/copy the internal leader state when transitioning
between Leader and IsolatedLeader.

Change-Id: If06996dccf083fd5d37757fd91fde2eb0eb82ea1
Signed-off-by: Tom Pantelis <>
6 years agoFix intermittent failure in testRemoveShardReplica 53/44953/2
Tom Pantelis [Wed, 31 Aug 2016 16:48:38 +0000 (12:48 -0400)]
Fix intermittent failure in testRemoveShardReplica

This test has failed on jenkins a few times:

java.lang.AssertionError: Shard cars is present
at org.opendaylight.controller.cluster.datastore.MemberNode.verifyNoShardPresent(
at org.opendaylight.controller.cluster.datastore.admin.ClusterAdminRpcServiceTest.testRemoveShardReplica(

The log output indicates member-2 hadn't re-joined with member-1 yet after it was
restarted. So when RemoveServer was sent to member-1 to remove member-2, it tried to
send the ServerRemoved message to the member-2 shard but it wasn't delivered and thus
the shard wasn't shut down and removed. To alleviate this I added a waitTillReady call
on member-2's config data store to ensure it has synced with the shard leader on

Change-Id: I8de9e585998d9f7b2ab8e4fd3f23c1ab222886cc
Signed-off-by: Tom Pantelis <>
6 years agoFix incorrect remove call in ShardManager 49/44949/1
Tom Pantelis [Wed, 31 Aug 2016 16:21:38 +0000 (12:21 -0400)]
Fix incorrect remove call in ShardManager

THe method onRemoveServerReply calls shardReplicaOperationsInProgress.remove
passing the ShardIdentifier however the shardReplicaOperationsInProgress
Set contains shard name Strings. We need to pass shardId.getShardName().

Change-Id: I3441fe8bbe88d65d20f8d9e8d572865f3cd381d8
Signed-off-by: Tom Pantelis <>
6 years agoBug 6535: Fix feature config pusher warning 69/44669/6
Tom Pantelis [Thu, 25 Aug 2016 13:52:35 +0000 (09:52 -0400)]
Bug 6535: Fix feature config pusher warning

The feature config pusher assumes config files installed by features
that have XML extension are CSS files and tries to parse them. On failure
it prints a warning but ignores it and moves on. Up till now we've only
had CSS XML files but we now have XML files related to the
clustered-app-config. To avoid the warning I added a check to see if the
root element is "snapshot" if JAXB parsing fails.

Change-Id: I877921afc13564f131f61a1eb8327db71d3638fe
Signed-off-by: Tom Pantelis <>
6 years agoBUG-5280: expose backing client actor reference 52/44852/1
Robert Varga [Thu, 25 Aug 2016 13:37:53 +0000 (15:37 +0200)]
BUG-5280: expose backing client actor reference

Exposing the actor directly allows us to retain only the context
without having to attach a specific behavior to implementation
utility classes.

Change-Id: I6fd26a23b08f4c7cb9d70d817aaf8deb44d55d88
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: make EmptyQueue public 51/44851/1
Robert Varga [Thu, 25 Aug 2016 12:13:43 +0000 (14:13 +0200)]
BUG-5280: make EmptyQueue public

Allow EmptyQueue to be used externally and correct its javadoc.

Change-Id: I4ae03095844ea235e735ffbdc48f974343a8e9a1
Signed-off-by: Robert Varga <>
6 years agoFix issue when AE leader differs from prior install snapshot leader 13/44813/3
Tom Pantelis [Mon, 29 Aug 2016 21:11:35 +0000 (17:11 -0400)]
Fix issue when AE leader differs from prior install snapshot leader

When a leader snapshot install is in progress, a follower doesn't process
AppendEntries and merely returns a reply with its last term/index. However,
if an install snapshot is initiated by a leader and there is more than one
chunk to send, it's possible for a leader change to occur prior to completing
sending all the chunks. When this happens, the new leader will begin sending
AppendEntries but the follower won't process them and make progress. We need
to clear the follower's snapshot state if the AppendEntries leaderId doesn't
match the prior install snapshot leaderId.

Change-Id: I4051bd064b6a20f4bcfe38b50656488fcb09274e
Signed-off-by: Tom Pantelis <>
6 years agoBUG-5280: fix a few warnings 55/44655/1
Robert Varga [Thu, 25 Aug 2016 08:44:00 +0000 (10:44 +0200)]
BUG-5280: fix a few warnings

This fixes javadoc warnings, addsa tiny bit of documentation
and corrects logging during recovery.

Change-Id: I13076ec052febb801a08c05adb7d8affca1fea82
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: add ExplicitAsk utility class 54/44654/1
Robert Varga [Thu, 25 Aug 2016 09:31:24 +0000 (11:31 +0200)]
BUG-5280: add ExplicitAsk utility class

Akka's support for Explicit Ask with Java functions
is broken and requires a workaround. This patch moves
the previous implementation site to a reusable place
and fixes a caller which was broken.

Change-Id: Idc0fc961b1808c23e01a4ae8c4eafdc93b7974f2
Signed-off-by: Robert Varga <>
6 years agoImplement toString in DataBroker impls 73/44573/2
Tom Pantelis [Tue, 23 Aug 2016 21:45:57 +0000 (17:45 -0400)]
Implement toString in DataBroker impls

PingPongDataBroker extends from ForwardingObject which forwards toString to the
delegate which is the ConcurrentDOMDataBroker. Unfortunately this hides the fact
that it's actually the PingPongDataBroker and has lead to confusion. Similar with the
BindingDOMDataBrokerAdapter. So I implemented toString to appropriately to refect the
true identity and forwarding nature of the classes.

Change-Id: I58b931590ca41bdb90bad31f6fedfe71466787ea
Signed-off-by: Tom Pantelis <>
6 years agoBUG-5280: Create AbstractProxyHistory class 77/44277/4
Robert Varga [Thu, 18 Aug 2016 14:05:32 +0000 (16:05 +0200)]
BUG-5280: Create AbstractProxyHistory class

Given the connection-oriented nature of SequencedQueue, we
really need to properly encapsulate various aspects of the client,
so we can perform proper state propagation, both during message
transmission and on reconnection.

This is a first step in that direction, which encapsulates client's
sendRequest() and self() methods at proper levels. It furthermore
makes state tracking in proxies consistent with state tracking in
their aggregate counterparts, hence each ProxyTransaction is guaranteed
to have an associated ProxyHistory.

Change-Id: I8c15b234ec813ac427e63a6e077ae17cde443be3
Signed-off-by: Robert Varga <>
6 years agoBug 6521: Fix ClassNotFoundException in sal-remoterpc-connector 09/44609/2
Tom Pantelis [Wed, 24 Aug 2016 14:17:38 +0000 (10:17 -0400)]
Bug 6521: Fix ClassNotFoundException in sal-remoterpc-connector

Introduced by The generated yang
files were importing org.opendaylight.controller.cluster. Now that the yang has been
removed we need to explicitly import the package into the bundle.

Change-Id: I049751f08f26bdceed1a6e8c2af7be940d92c591
Signed-off-by: Tom Pantelis <>
6 years agoBug 5700 - Backwards compatibility of sharding api's with old api's 13/43613/11
Jakub Morvay [Wed, 10 Aug 2016 12:08:36 +0000 (14:08 +0200)]
Bug 5700 - Backwards compatibility of sharding api's with old api's

Implementation of controller DOMDataBroker interface which delegates
calls to shard aware implementation of md-sal DOMDataBroker

Change-Id: I5694da6d660453ed6a0382006df808cc321d4130
Signed-off-by: Jakub Morvay <>
6 years agoBUG-5280: move proxy instantiation to AbstractClientHistory 76/44276/4
Robert Varga [Thu, 18 Aug 2016 13:15:12 +0000 (15:15 +0200)]
BUG-5280: move proxy instantiation to AbstractClientHistory

Histories should be the synchronization point for accessing
per-cookie proxies. Move instantiation code, making cookie/history
mapping internal to AbstractClientHistory.

Change-Id: I512e93d72b682668790a5dd213112d772143f045
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: separate request sequence and transmit sequence 33/43333/28
Robert Varga [Mon, 8 Aug 2016 15:04:54 +0000 (17:04 +0200)]
BUG-5280: separate request sequence and transmit sequence

Clean up the confusion in sequence numbers. There are actually
two sequences:
- logical request sequence, which indicates the order of requests
  in which they should be applied to the target entity. It is
  assigned by logic emitting the request.
- transmit sequence within a connection to the backend, as
  initiated by ConnectClientRequest. It is assigned by SequencedQueue
  as it is transmitting requests and reset when a new connection
  to the backend is made.

This requires establishing the concept of a session, which is
a single conversation between frontend and backend. It is severed
whenever the frontend times out and re-established when the leader
is found and it responds with ConnectClientSuccess.

The sending of ConnectClientRequest is not done via the queue,
as it is part of backend resolution process. Since this is not
a performance-critical path, we use simple Patterns.ask() to
send the request and get completion notification -- which we then
translate to ShardBackendInfo.

ConnectClientSuccess gives us backend-preferred version and
backend-specified cap on the number of outstanding messages then
it can handle concurrently. This maximum is used to limit the
transmit path of SequencedQueue, so that it does not attempt
to send more requests at any given time.

Internal queue for unsent requests is kept unbounded for now,
subject to a Semaphore-driven throttle in a follow-up patch.

Change-Id: I61663073bf6632c1ed8c036dee37f1ac39cf7794
Signed-off-by: Robert Varga <>
6 years agoRemove deprecated CSS files - sal-remoterpc-connector 53/44553/3
Alexis de Talhouët [Tue, 23 Aug 2016 13:44:25 +0000 (09:44 -0400)]
Remove deprecated CSS files - sal-remoterpc-connector

Change-Id: I2d163486e15ca7eaededb45194645f24f00bfca2
Signed-off-by: Alexis de Talhouët <>
6 years agoFix issues when persistence enabled 21/44521/3
Tom Pantelis [Mon, 22 Aug 2016 21:32:59 +0000 (17:32 -0400)]
Fix issues when persistence enabled

When persistence is dynamically enabled, we start persisting subsequent log
entries which causes issues on restart due to a gap in journal indexes. We need to
persist a snapshot with the current state to avoid this.

Also if persistence is disabled, we still persist ReplicatedLogEntry instances that
have a PersistentPayload, of which ServerConfigurationPayload is currently the only one.
This also can cause gaps in the persisted journal indexes which cause issues if
persistence is later enabled. To avoid this, we really shouldn't persist
ReplicatedLogEntry instances at all if data persistence is disabled since we don't
add them to the in-memory journal on recovery anyway - we just recover and apply the
ServerConfigurationPayload. Instead we should persist just the

Change-Id: Ief78d68423b33aac1649220a36d32ff50f493eb7
Signed-off-by: Tom Pantelis <>
6 years agoTurn off visibility of GlobalBundleScanningSchemaServiceImpl#start() 49/44549/1
Alexis de Talhouët [Tue, 23 Aug 2016 12:26:32 +0000 (08:26 -0400)]
Turn off visibility of GlobalBundleScanningSchemaServiceImpl#start()

Since the start() method is only used in the createInstance(), it
should be private and not exposed.

Change-Id: I0264d0a66bbfb2536bc4d6c57f27f15584ddfabb
Signed-off-by: Alexis de Talhouët <>
6 years agoRevert "Fix feature config pusher warning" 16/44516/1
Tom Pantelis [Mon, 22 Aug 2016 20:05:42 +0000 (16:05 -0400)]
Revert "Fix feature config pusher warning"

This reverts commit 64e760caf2a5d223cca7598b3ff8941ac37c7a96. Unfortunately some projects don't follow the convention of installing their CSS xml files under etc/opendaylight/karaf.

Change-Id: I6194035399eddc1d66cadaa7f9b0964b9a46d92b
Signed-off-by: Tom Pantelis <>
6 years agoFix feature config pusher warning 85/44185/2
Tom Pantelis [Wed, 17 Aug 2016 17:12:31 +0000 (13:12 -0400)]
Fix feature config pusher warning

The feature config pusher assumes config files installed by features
that have XML extension are CSS files and tries to parse them. On failure
it prints a warning but ignores it and moves on. Up till now we've only
had CSS XML files but we now have XML files related to the
clustered-app-config. To avoid the warning I added an additional check
for "opendaylight/karaf" present in the file path. To be complete we would
open the file and inspect the first element but I didn't think that was worth
the effort since all current CSS files are put under opendaylight/karaf and
ODL is moving away from CSS.

Change-Id: I1f0dcda9efa14db330a72e455fe4756c8fbfcaab
Signed-off-by: Tom Pantelis <>
6 years agoMake DataTreeCandidatePayload MigratedSerializable 09/44109/2
Robert Varga [Tue, 16 Aug 2016 22:44:41 +0000 (00:44 +0200)]
Make DataTreeCandidatePayload MigratedSerializable

This class has been superseded and we want to purge it from
everywhere. Mark it as MigratedSerializable, which will trigger
a snapshot to be taken.

Change-Id: I1613d3cc2c359296718a20a4263ba848259f46d3
Signed-off-by: Robert Varga <>
6 years agoTake snapshot after recovery on migrated messages 47/43747/8
Tom Pantelis [Thu, 11 Aug 2016 01:19:59 +0000 (21:19 -0400)]
Take snapshot after recovery on migrated messages

Modified RaftActorRecoverySupport to capture and persist a snapshot
after recovery when there are migrated messages recovered. It utilizes
the new MigratedSerializable interface.

I also created equivalent classes in the persisted packages for
UpdateElectionTerm, DeleteEntries and ApplyJournalEntries that implement
MigratedSerializable and use the Externalizable proxy pattern. The
existing classes were deprecated and readResolve to the new classes.

Change-Id: Ia2e664de9ffd59991c49160424b13bc8ca0bfcbf
Signed-off-by: Tom Pantelis <>
6 years agoFix FinalModifierException due to BindingToNormalizedNodeCodec final 74/44274/2
Tom Pantelis [Thu, 18 Aug 2016 14:31:00 +0000 (10:31 -0400)]
Fix FinalModifierException due to BindingToNormalizedNodeCodec final

The BindingToNormalizedNodeCodec class is final which prevents blueprint from
being able to create a proxy instance when advertising as a service. This is
is OK as it will use the actual instance but it logs an exception:

2016-08-11 23:03:18,215 | INFO  | rint Extender: 2 | ServiceRecipe
| 15 - org.apache.aries.blueprint.core - 1.6.1 | Unable to create a
proxy object for the service .component-2 defined in bundle
org.opendaylight.controller.sal-binding-broker-impl/1.5.0.SNAPSHOT with
id. Returning the original object instead.
org.apache.aries.proxy.FinalModifierException: The class
is final.

Although it's logged at INFO level we should avoid it.

Changing BindingToNormalizedNodeCodec to non-final fixes the exception however it causes
an error when it tries to create the proxy b/c some methods are final. To avoid this, weneed to avoid the proxy creation altogether so I added a method to
BindingToNormalizedNodeCodecFactory to advertise the OSGi service instead of using
the blueprint <service> element.

Note: it's not good practice to advertise a service with the actual class but it's
needed with BindingToNormalizedNodeCodec for backwards compatibility with CSS modules
that inject the BindingToNormalizedNodeCodec instance and not its interfaces. The
BindingToNormalizedNodeCodec CSS service identity is deprecated and will/should go away in the future.

Change-Id: I96b6cb8b030de39808de17142d79f8bbd09bf735
Signed-off-by: Tom Pantelis <>
6 years agoBug 6278: Move opendaylight-karaf-empty to odlparent 77/44077/4
Ryan Goulding [Tue, 9 Aug 2016 21:00:26 +0000 (17:00 -0400)]
Bug 6278: Move opendaylight-karaf-empty to odlparent

Move opendaylight-karaf-empty from controller to odlparent. As with
moves of other artifacts, the following process will happen:

1) Copy opendaylight-karaf-empty into odlparent and adjust the groupId.
   The patch that handled the copy work is:

2) Change opendaylight-karaf-empty in controller to derive from odlparent's
   opendaylight-karaf-empty.  This is handled in this patch.

3) Change references to use the artifact added in #1.

4) Deprecate and remove controller's opendaylight-karaf-empty.

This patch just handles #2, re-parenting the existing controller
opendaylight-karaf-empty such that it derives from odlparent's
opendaylight-karaf-empty (added in #1).

Change-Id: Ifbfedd8a06f5f03900277d005906af83220707cc
Signed-off-by: Ryan Goulding <>
6 years agoDataBrokerTestModule: use AbstractDataBrokerTest without inheritance 45/43645/3
Michael Vorburger [Wed, 10 Aug 2016 15:22:30 +0000 (17:22 +0200)]
DataBrokerTestModule: use AbstractDataBrokerTest without inheritance

While starting to write JUnit tests (for functional components) with the
AbstractDataBrokerTest, I found myself not liking its design which
forces one to use a class SomeTest extends AbstractDataBrokerTest...

Modern JUnit tests should be written by composition, not enforced
inheritance.  Thus this new DataBrokerTestModule allows one to obtain a
DataBroker suitable for Tests from anywhere.

The *(Test)Module naming convention is obviously inspired by a DI point
of view (think Spring Framework's @Configuration classes, or Google
Guice or Dagger (yay!) *Module classes - which is exactly what this
really is - a particular way to obtain an instance of an
implementationof some service (here a DataBroker) in a certain
environment (here for tests).

The internal implementation of DataBrokerTestModule could be changed
later; I guess ideally what's in AbstractDataBrokerTest could go into
DataBrokerTestModule and AbstractDataBrokerTest could use it, but for
now I'm too lazy to change that, as this does the trick nicely.  Later
implementation changes would be transparent to users of

Change-Id: I9641f527bbc0cb92732f2e513cdd64cc6a837200
Signed-off-by: Michael Vorburger <>
6 years agoLimit visibility of ShardDataTree methods 06/43006/12
Robert Varga [Wed, 3 Aug 2016 00:13:36 +0000 (02:13 +0200)]
Limit visibility of ShardDataTree methods

Reduce visibility to package-private and add
@VisibleForTesting where this is not possible.

Some of the methods are not converted, as they are used
by the EOS implementation.

Change-Id: Ie7472fec4439e69479d097520fd81be9ea16a88c
Signed-off-by: Robert Varga <>
6 years agoBUG-865: remove String-based getDataChildByName() 88/43588/3
Martin Ciglan [Wed, 10 Aug 2016 08:58:08 +0000 (10:58 +0200)]
BUG-865: remove String-based getDataChildByName()

This is follow-up patch to reflect Yangtools changes
and needs to be merged after

Change-Id: Icb66e8096cb77589de3ee89a87917a528e8d0dc0
Signed-off-by: Martin Ciglan <>
6 years agoAddress review comments 75/43775/2
Tom Pantelis [Fri, 12 Aug 2016 06:00:36 +0000 (02:00 -0400)]
Address review comments


Change-Id: If054abf5dce4207fe536beeef15ffaad61b63074
Signed-off-by: Tom Pantelis <>
6 years agoBUG-5280: fix compilation after unrebased merge 78/43778/1
Robert Varga [Fri, 12 Aug 2016 12:03:54 +0000 (14:03 +0200)]
BUG-5280: fix compilation after unrebased merge

Merging the last batch of patches resulted in a mismatch
between newly-introduced tests and method movement. Fix
that up.

Change-Id: I326ad06ff07b902987e397f13f8b0afdb8c45231
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: add maxMessages field to ConnectClientSuccess 28/43328/9
Robert Varga [Mon, 8 Aug 2016 14:31:44 +0000 (16:31 +0200)]
BUG-5280: add maxMessages field to ConnectClientSuccess

This field will act as a hint on how many messages may
be queued by the frontend towards the backend at any
given time.

Change-Id: Ibb8bbe2af9595bc0ecee090acea35aa78a9250b7
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: add FrontendMetadata 24/43124/15
Robert Varga [Wed, 3 Aug 2016 00:52:20 +0000 (02:52 +0200)]
BUG-5280: add FrontendMetadata

This patch adds the frontend tracking abilities for followers.
It also defines the corresponding ShardDataTreeSnapshotMetadata
for use with persistence.

Change-Id: I7e2c6755c3389dcb5284f17a9c6076fb9e7ac95e
Signed-off-by: Robert Varga <>
Signed-off-by: Vaclav Demcak <>
6 years agoBUG-5280: persist metadata in snaphots 74/42974/19
Robert Varga [Mon, 1 Aug 2016 21:20:11 +0000 (23:20 +0200)]
BUG-5280: persist metadata in snaphots

This patch adds the wiring in ShardDataTree to persist
various pieces of metadata in a snapshot. It also includes
metadata recovery from a snapshot.

In order to make this work, this patch centralizes all
actual payload and snapshot handling within the ShardDataTree
by introducing explicit entrypoints for each avenue through
which data can be introduced.

Change-Id: Ibc15bd152bd44dd583d67bb7fc61bc8f3086df30
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: split out cds akka client substrate 21/43321/9
Robert Varga [Mon, 8 Aug 2016 13:54:20 +0000 (15:54 +0200)]
BUG-5280: split out cds akka client substrate

This patch splits out the baseline frontend client
into a separate package.

Change-Id: I2d8ca8b81f29a45dd8c30f3bef467fcda94d4887
Signed-off-by: Robert Varga <>
6 years agoMove MessageTrackerTest 22/43322/7
Robert Varga [Mon, 8 Aug 2016 14:13:42 +0000 (16:13 +0200)]
Move MessageTrackerTest

This moves the test to sal-clustering-commons, so we end
up testing in its home artifact.

Change-Id: Ie001bee6de92381ab6140a3a54e31c854a46ae1d
Signed-off-by: Robert Varga <>
6 years agoBug 5450: Query akka cluster state on Follower ElectionTimeout 65/43265/6
Tom Pantelis [Fri, 5 Aug 2016 20:10:58 +0000 (16:10 -0400)]
Bug 5450: Query akka cluster state on Follower ElectionTimeout

Added changes to query the akka ClusterState to see if the leader is
actually unreachable or not Up on ElectionTimeout. If not, Follower
reschedules election timer and stays as Follower.

Change-Id: I3a054a82edbe975ad9e27c4d208083b19b392d2d
Signed-off-by: Tom Pantelis <>
6 years agoChange InstallSnapshot and reply to use Externalizable Proxy 38/42638/4
Tom Pantelis [Tue, 26 Jul 2016 22:36:06 +0000 (18:36 -0400)]
Change InstallSnapshot and reply to use Externalizable Proxy

This makes InstallSnapshot cleaner with no public no-arg constructor.

I also removed the InstallSnapshot protobuff message. In addition,
SerializableUtils is no longer needed as there's no more protobuff

Change-Id: I17aa4f7195cf09b798daee5587bbf50ccbc4bff0
Signed-off-by: Tom Pantelis <>
6 years agoBUG-6111: fix a thinko 28/43628/1
Robert Varga [Wed, 10 Aug 2016 09:24:31 +0000 (11:24 +0200)]
BUG-6111: fix a thinko

Failure to initialize isOpen leads to the codepath never being

Change-Id: I20f1b76c9ada581edc1c92c61447fd97d0d1b2ea
Signed-off-by: Robert Varga <>
(cherry picked from commit 01ca6a43914fb8dd27a24da7476b835fc2570e40)

6 years agoMove ServerConfigurationPayload to cluster.raft.persisted 66/43266/7
Robert Varga [Fri, 5 Aug 2016 17:44:14 +0000 (19:44 +0200)]
Move ServerConfigurationPayload to cluster.raft.persisted

This introduces its mirror copy and modifies the old class
so that it readResolve()s to the new class. It also adjusts
all users to use the new class.

The new class uses Externalizable proxy pattern to allow the
class itself be evolved without breaking compatibility. Also
NoOpPayload is retrofitted this way, which makes all subclasses
of Payload not have their serialization format tied to Payload

Change-Id: I26010a9e1438dbc4cb1822e1c4dbb51e2b6e538e
Signed-off-by: Robert Varga <>
6 years agoBug 6278: Switch to use odlparent's karaf-parent 44/43144/3
Ryan Goulding [Thu, 4 Aug 2016 09:24:19 +0000 (05:24 -0400)]
Bug 6278: Switch to use odlparent's karaf-parent

Switch archetypes so they lay down the correct karaf-parent implementation.

Change-Id: Ib9aa057f45141579a0d2bc895d373bd2882f975c
Signed-off-by: Ryan Goulding <>
6 years agoBump ietf versions to ...10-SNAPSHOT 40/43540/2
Thanh Ha [Tue, 9 Aug 2016 17:43:50 +0000 (13:43 -0400)]
Bump ietf versions to ...10-SNAPSHOT

Bump versions according to:

Change-Id: I3038eaf68217d131bf867b1fcb2abb3abbf76663
Signed-off-by: Thanh Ha <>
6 years agoBug 6348 : car:stop-stress-test RPC to return success & failure counters 59/43259/5
Sai MarapaReddy [Tue, 19 Jul 2016 20:49:21 +0000 (13:49 -0700)]
Bug 6348 : car:stop-stress-test RPC to return success & failure counters

Current RPC car:stop-stress-test doesn't return how many
cars are created or failed. Adding success and failure counters
will help user to determine the number of cars created or failed
during the the process of creation of car tests using
car:stress-test. This patch enhances car:stop-stress-test RPC.

Change-Id: Iff054c8210ce49f06b4fa96ca5a437d9b82deddb
Signed-off-by: Sai MarapaReddy <>
Author: Sai MarapaReddy <>

6 years agoFix ietf-yang-types version 68/43468/1
Thanh Ha [Tue, 9 Aug 2016 04:54:26 +0000 (00:54 -0400)]
Fix ietf-yang-types version

The version bump script messed up bumping the ietf version. Fixing it
with this patch.

Change-Id: I9b3975582a3a2fe35ab5f4509b6d3a70bf5ca243
Signed-off-by: Thanh Ha <>
6 years agoFix relative paths for mdsal-it-parent 64/43464/1
Anil Belur [Tue, 9 Aug 2016 01:39:55 +0000 (11:39 +1000)]
Fix relative paths for mdsal-it-parent

Change-Id: Id321a86500216a8cf0ebe5ce7ebb698717996c7c
Signed-off-by: Anil Belur <>
6 years agoBump versions by 0.1.0 for next dev cycle 09/43409/1
Thanh Ha [Mon, 8 Aug 2016 21:50:13 +0000 (17:50 -0400)]
Bump versions by 0.1.0 for next dev cycle

Change-Id: I9c4a4b1e6d0e101392fb19dc68f814e30de4fa5c
Signed-off-by: Thanh Ha <>
6 years agoBUG-6111: implement PingPongTransactionChain cancelation 58/43058/3
Robert Varga [Wed, 3 Aug 2016 15:15:57 +0000 (17:15 +0200)]
BUG-6111: implement PingPongTransactionChain cancelation

This patch implements transaction cancelation in PingPongDataBroker,
which has slightly different semantics -- if a transaction is canceled
while being in a batch, proper isolation of the batch is maintained
and after preceding batch completes, the transaction chain is aborted.

Since there is no transaction isolation within a batch, this is the
only course of action we can take.

Change-Id: I0058503165dbfba8748a17a9ef9272265f4bc1c9
Signed-off-by: Robert Varga <>
6 years agoAdd PMD exclusion for config-generated files 42/43242/2
Robert Varga [Fri, 5 Aug 2016 15:09:31 +0000 (17:09 +0200)]
Add PMD exclusion for config-generated files

Unfortunately PMD does not support wildcard
root exclusions, hence we have to match odlparent
configuration and extend it.

Change-Id: I4bc7a1b8c25b75cb5b348fb2a16f0e5b2c111359
Signed-off-by: Robert Varga <>
6 years agoBug 5504: Add PreLeader raft state 28/42728/4
Tom Pantelis [Wed, 27 Jul 2016 19:52:53 +0000 (15:52 -0400)]
Bug 5504: Add PreLeader raft state

The following scenario can result in a "store tree and candidate base
differ" IllegalStateException on commit:

A follower receives a replicate and adds it to the log, say at index 1,
but the leader transfers or dies before committing and applying it to the
state. The follower becomes leader and when the next tx is applied, log
index 2, it has to first apply all log entries from the previous term that
hadn't been committed yet, in this case index 1. Since we got consensus for
index 2 that means index 1 has also been replicated to a majority. Therefore
ApplyState is sent for index 1 and then index 2. However index 1 is applied
as a "foreign" candidate while index 2 is in the pre-commit state. When
index 2 is applied the commit fails.

To prevent this scenario, we introduce a new raft state, PreLeader,
which is transitioned to from Candidate if there are uncommitted
entries, ie commit index < last log index. The PreLeader state performs all
the duties of Leader with the added behavior of attempting to commit all
uncommitted entries from the previous leader's term. Raft does not allow a
leader to commit entries from a previous term by simply counting replicas -
only entries from the leader's current term can be committed (§5.4.2). Rather
then waiting for a client interaction to commit a new entry, the PreLeader
state immediately appends a no-op entry (NoopPayload) to the log with the
leader's current term. Once the no-op entry is committed, all prior entries
are committed indirectly. Once all entries are committed, ie commitIndex matches
the last log index, it switches to the normal Leader state.

The PreLeader state is considered an inactive leader state and thus
client transactions are delayed until it transitions to Leader.

Change-Id: I20a541de0eba9b0075b9952dc6d5808943b7bb8f
Signed-off-by: Tom Pantelis <>
6 years agoBUG-5280: expand ShardDataTree to cover transaction mechanics 97/42497/27
Robert Varga [Mon, 25 Jul 2016 18:59:37 +0000 (20:59 +0200)]
BUG-5280: expand ShardDataTree to cover transaction mechanics

A chunk of ShardCommitCoordinator should actually be implemented
by ShardDataTree. This includes transaction queueing, commit timers,
interaction with user cohorts and persistence.

This patch implements the relevant operations in an message-agnostic,
callback-driven way.

Fix: ShardDataTreeTest (missing ShardStat MBean)

Change-Id: I353bacce8245df85c5f4d6b4cc0ce5416f2f0337
Signed-off-by: Robert Varga <>
Signed-off-by: Vaclav Demcak <>
Signed-off-by: Tom Pantelis <>
6 years agoFix relativePath declaration 60/43060/2
Robert Varga [Wed, 3 Aug 2016 16:03:01 +0000 (18:03 +0200)]
Fix relativePath declaration

Since karaf-parent's parent is outside of controller,
relativePath has to be empty.

Change-Id: I491c73f2d42b8d5f3e159625f8ba01fdadd32497
Signed-off-by: Robert Varga <>
6 years agoReturn shortened string from TransactionIdentifier.toString 14/42314/3
Tom Pantelis [Fri, 22 Jul 2016 01:30:36 +0000 (21:30 -0400)]
Return shortened string from TransactionIdentifier.toString

For debug logging we need a shortened string for better readability and
grepping. The standard toString is way too long. I changed toString to a
similar compact form that we had before. adding in the frontend generation id
and type, eg


Change-Id: I942eaaa0e8ceedf42eed964f2a2e3a76d8c09806
Signed-off-by: Tom Pantelis <>
6 years agoEnable akka WeaklyUp feature 99/42799/3
Tom Pantelis [Fri, 29 Jul 2016 18:33:27 +0000 (14:33 -0400)]
Enable akka WeaklyUp feature

By enabling allow-weakly-up-members, akka will allow new nodes to join a
cluster if there are unreachable nodes. However, this only pertains to
new nodes that weren't previously in the cluster. Unfortunately it
doesn't pertain to node restarts where a node was in the cluster then
attempts to re-join with a new incarnation, which is what we really want.
Despite that, it will at least work for new nodes so I think it's worth
enabling. Akka might be further enhanced to broaden WeaklyUp to include
new incarnations (there's requests for that).

I also changed the ShardManager to handle MemberWeaklyUp events in
the same manner as MemberUp.

Change-Id: I5cf6c1967162b8a9bc6ffb59d34a50560699e4ca
Signed-off-by: Tom Pantelis <>
6 years agoBug 6278: Copy karaf-parent from controller to odlparent 49/42649/5
Ryan Goulding [Fri, 22 Jul 2016 07:52:08 +0000 (03:52 -0400)]
Bug 6278: Copy karaf-parent from controller to odlparent

As discussed in the MD-SAL call, there is an architectural need to move
karaf-parent from the controller project to the odlparent project.  This
is particularly useful for karaf upgrades, since right now a bump in karaf
version within odlparent requires a rebuild of controller to reflect the
change in karaf-parent, and our build jobs are not set up to support such
a process.

The move process will be handled in multiple steps:

1) Copy karaf-parent, karaf-branding and opendaylight-karaf-resources to
odlparent.  All three of these should belong in odlparent.  All three must
be moved since karaf-parent depends on the latter two artifacts.  Since
controller depends on odlparent (and not the other way around), they must
be moved upstream to odlparent.

2) Have controller's karaf-parent derive from odlparent's karaf-parent.
This preserves the ability for downstream consumers to derive from the
controller karaf-parent in the interim, while allowing changes to odlparent's
karaf-parent to be recognized since controller does not need to be rebuilt.

This also involves removing karaf-branding and opendaylight-karaf-resources
from the controller project, since they are no longer needed.  There are two
consumers that need to be patched:

3) Change all downstream projects to utilize odlparent's karaf-parent.  This
is future work and will be done in several patches.

4) Remove controller's karaf-parent once we feel all downstream consumers
are using the odlparent's karaf-parent.

Change-Id: Ib42ff5212bbfb93883346a19855544df4fb06d61
Signed-off-by: Ryan Goulding <>
6 years agoDo not use ShardDataTree in PruningDataTreeModificationTest 75/42975/5
Robert Varga [Tue, 2 Aug 2016 13:33:25 +0000 (15:33 +0200)]
Do not use ShardDataTree in PruningDataTreeModificationTest

This test requires on a DataTree, hence use that.

Change-Id: I37697121f6686cdfe6b1d71ca87ff79281619532
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: add client connect messages 66/42866/7
Robert Varga [Sun, 31 Jul 2016 21:55:28 +0000 (23:55 +0200)]
BUG-5280: add client connect messages

When a frontend is attempting to re-establish communication
with the backend it sends its coordinates and various other
information to a backend.

Sending ConnectClientRequest initiates a handshake, to which
the backend will respond either with a failure, or with an
adjusted ConnectClientSuccess.

Change-Id: I58ba9a2103f80e528654222f82f07416f7d7815e
Signed-off-by: Robert Varga <>
6 years agoHandle DeleteSnapshots response messages 00/42800/3
Tom Pantelis [Fri, 29 Jul 2016 19:23:23 +0000 (15:23 -0400)]
Handle DeleteSnapshots response messages

This is a follow-up to
I didn't think deleteSnapshots returned a response but it does - I see
a warning for unhandled DeleteSnapshotsSuccess. I added handling for
DeleteSnapshotsSuccess and DeleteSnapshotsFailure. For the latter I log
a warning but don't fail the actor.

Change-Id: Ibb41e5124eb22530f98a5ef958abffc556dea4cf
Signed-off-by: Tom Pantelis <>
6 years agoFix missing LeaderStateChanged event 97/42597/4
Tom Pantelis [Tue, 26 Jul 2016 15:55:09 +0000 (11:55 -0400)]
Fix missing LeaderStateChanged event

In RaftActor, the logic to detect a leader state change compares the last
valid leader Id with the current behavior leader Id. Consider the
following leader Id change sequence:

  "member-1" -> null (goes leaderless)
  null -> "member-1" (member-1 becomes leader again)

The first state change will send a LeaderStateChanged event to the
ShardManager with null leader Id causing the ShardManager to clean its
primary shard info cache. However for the second state change, no
LeaderStateChanged event is sent b/c the new leader Id is the same as
the last valid/non-null leader Id. Therefore transactions fail due to no
shard leader.

I changed it to use the last leader Id (null or not) for the comparison
so every state change is detected.

Change-Id: I060872d4712e040b60acfc998914b394a40943af
Signed-off-by: Tom Pantelis <>
6 years agoImprove leader election convergence 69/42969/3
Tom Pantelis [Tue, 2 Aug 2016 02:23:33 +0000 (22:23 -0400)]
Improve leader election convergence

When 2 nodes startup with the first node's log behind the second node's,
it usually takes several election rounds to converge - I've seen
anywhere from 40 s to 3 min, depending on timing. What happens is that
the first node goes to Candidate first but it's RequestVote is rejected
by the seconds node. Shortly after the seconds node goes to Candidate -
the term is higher than the first which causes the first node to go back
to Follower. However it doesn't respond to the RequestVote. Then the
first node goes to Candidate and the cycle repeats. Eventually, due to
the election variance, the seconds node times out first and the second
node process the RequestVote and grants it. But it can take more than 10

We can improve the convergence by allowing a Candidate to process and
respond to RequestVote when the sender's term is greater. It still
transitions to Follower as per the raft rules. The raft paper does not
say whether or not a Candidate can/should process a RequestVote in this
case but it seems to make sense. With this change, the first RequestVote
sent by the second node is granted and it converges quickly.

Change-Id: If9416ddf7bf0dfc1220a169be4174f440626a0dd
Signed-off-by: Tom Pantelis <>
6 years agoAlleviate premature elections in followers 64/42564/7
Tom Pantelis [Tue, 26 Jul 2016 04:07:02 +0000 (00:07 -0400)]
Alleviate premature elections in followers

If a follower actor is busy or some non-leader messages take longer to process,
leader messages may get backed up enough to cause the election timer to
expire, thereby resulting in an unwanted election and leader disruption. To
alleviate this scenario, I added a Stopwatch to keep the last time a leader
message was received, ie when a leader message is received it restarts
the Stopwatch. When ElectionTimeout is received, it checks if the
elapsed time of the Stopwatch has exceeded the election timeout
interval. Therefore if leader messages were occurring during the
election timeout interval but were delayed, they will be processed
before the ElectionTimeout message and restart the Stopwatch such that the
elapsed time will/should be less than the election timeout interval by the
time ElectionTimeout is received (unless the last leader message happened to
take longer than the election timeout interval).

There are cases where ElectionTimeout is manually sent to force an
election timeout (eg during leadership transfer). In these cases we
don't want to check the Stopwatch so I added an explicit TimeoutNow
message to distinguish the 2 messages.

Change-Id: I6b745288040da2fdcef1d29cb5ffc482c9e66003
Signed-off-by: Tom Pantelis <>
6 years agoBUG-5280: centralize ShardSnapshot operations 85/42785/11
Robert Varga [Fri, 29 Jul 2016 13:58:49 +0000 (15:58 +0200)]
BUG-5280: centralize ShardSnapshot operations

Current shard snapshotting mechanism does not allow for evolution
of the snapshot contents and contains only the root node. Also
the serialization and deserialization operations are scattered
in multiple places, making coordinated changes a bit troublesome.

This patch introduces a versioned snapshot abstraction and moves
serdes operations into a single place. A new serialization format
is introduced, which holds the root node and some additional
metadata. No concrete metadata is defined in this patch, but this
will be used to transfer frontend protocol state from shard leader
to shard follower.

It also moves the act of creating a snapshot into ShardDataTree
and creates a dedicated actor to handle the snapshotting task,
which is used for all snapshot requests for a particular Shard.
Also makes the actor message internal to the ShardSnapshotActor,
providing a convenience method to create and dispatch it.

Change-Id: I6d9680b6ef08672c363092a649255013980c0bd6
Signed-off-by: Robert Varga <>