6 years agoBUG-7594: Rework sal-remoterpc-connector messages 29/50629/34
Robert Varga [Wed, 18 Jan 2017 15:01:21 +0000 (16:01 +0100)]
BUG-7594: Rework sal-remoterpc-connector messages

This breaks compatibility by using DOMRpcIdentifier directly
in transferred messages. Since we are breaking compatibility, we can
also rework the messages and their locations, limiting their visiblity
and features (such as Serializable).

RoutingTable no longer uses RouteIdentifier, but rather relies
on DOMRpcIdentifier, which is serialized using
NormalizedNodeDataInput/Output primitives, so the serialization
format is not dependent on the package DOMRpcIdentifier comes from
and can be compatibly switched to mdsal-provided one.

Change-Id: Idf083f9d288be9c9684c7e8e8bd99fbaff0ad4ce
Signed-off-by: Robert Varga <>
Signed-off-by: Tomas Cere <>
6 years agoBug 7521: Convert install snapshot chunking to use streams 44/50744/7
Tom Pantelis [Fri, 20 Jan 2017 20:32:37 +0000 (15:32 -0500)]
Bug 7521: Convert install snapshot chunking to use streams

On the leader side, converted LeaderInstallSnapshotState to use
the InputStream from the ByteSource instead of a ByteString to
chunk the data.

On the follower side, converted the SnapshotTracker, which is used
to reassemble the install snapshot chunks, to write the chunks to
an OutputStream instead of a ByteString. Currently a
ByteArrayOutputStream is used by will be changed to a
FileBackedOutputStream in a subsequent patch.

Change-Id: I7a16ad5d44a530e260aa332d91145fbc3fb95f5f
Signed-off-by: Tom Pantelis <>
6 years agoBug 7521: Convert Snapshot to store a State instance 72/50572/9
Tom Pantelis [Tue, 17 Jan 2017 18:24:28 +0000 (13:24 -0500)]
Bug 7521: Convert Snapshot to store a State instance

Created a new Snapshot class that stores the state as a Serializable
State instance instead of a byte[] to lay the groundwork for handling
snapshots > 2G. In this manner, State implementations can serialize
their data directly to the ObjectOutputStream.

The previous Snapshot class was deprecated but, unfortunately, it
can't simply "read resolve" to the new class on de-serialization
as the deserialization code doesn't know how to convert the data
from byte[] to a State instance. Therefore a new method,
deserializePreCarbonSnapshot, was added to RaftActorRecoveryCohort.

The introduction of the State interface necessitated changes throughout
the snapshot capture and snapshot recovery code paths. The Shard
implementations were also changed accordingly.

For follower snapshot install, an optional OutputStream is also passed
to createSnapshot. The new API contract is: if the OutputStream is present,
the implementation must serialize its state to the OutputStream and return
the stream instance in the CaptureSnapshotReply along with the snapshot
State instance. The snapshot State is serialized directly to the snapshot
store while the OutputStream is used to send the state data to follower(s)
in chunks. The deserializeSnapshot method is used to convert the serialized
data back to a State instance on the follower end. The serialization for
snapshot install is passed off so the cost of serialization is not charged
to the raft actor's thread.

The OutputStream is converted to a ByteSource when passed to AbstractLeader.
AbstractLeader still converts it to a byte[] but will be changed to use
the ByteSource's InputStream to chunk the data in a subsequent patch.

Also the SnapshotManager currently passes a ByteArrayOutputStream to
createSnapshot but this will be changed to a FileBackedOutputStream in a
subsequent patch.

Change-Id: I2ea30870b54478d7ef5669335ca4b444539f8d56
Signed-off-by: Tom Pantelis <>
6 years agoBUG-7556: update version tracking 22/50622/22
Robert Varga [Wed, 18 Jan 2017 15:01:21 +0000 (16:01 +0100)]
BUG-7556: update version tracking

This patch adds better version tracking, so it does not rely
on calendar time, but rather is monotonically increasing.

The 64bit version field is logically split into an incarnation
number (31 bits) and a version number (32 bits).

Change-Id: Ie0e1f4089cc1ee582037982d9837490348158975
Signed-off-by: Robert Varga <>
6 years agoAbstractConcurrentDataBrokerTest @deprecate-s the AbstractDataBrokerTest 86/51486/3
Michael Vorburger [Mon, 6 Feb 2017 23:23:31 +0000 (00:23 +0100)]
AbstractConcurrentDataBrokerTest @deprecate-s the AbstractDataBrokerTest

This is take #2 on a similar earlier attempt which made the same change
directly in AbstractDataBrokerTest, which broke some downstrean tests,
which were BADLY WRITTEN (!) because the assumed single threaded direct
execution (which DataBroker does not guarantee). has full background

Bug: 7538
Change-Id: I05ac3525bdcf1ab9c99dfa15b98e090848d0fddc
Signed-off-by: Michael Vorburger <>
6 years agoodl-config-persister feature clean-up 63/51563/1
Stephen Kitt [Wed, 8 Feb 2017 15:43:02 +0000 (16:43 +0100)]
odl-config-persister feature clean-up

This removes all the bundles which are pulled in transitively. The
resulting feature file is identical, but the POM only declares the
bundles we really care about (which ones are they?).

This is probably controversial, and is one possible variant of three
(see the other patches in the same topic).

Change-Id: Ib57241d792cb494f2a3e967bc76e23cdb4fa9bec
Signed-off-by: Stephen Kitt <>
6 years agoRe-enable transitive dependency search 59/51559/2
Robert Varga [Wed, 8 Feb 2017 13:22:10 +0000 (14:22 +0100)]
Re-enable transitive dependency search

This is no longer needed with odlparent patches.

Change-Id: I960483a9f097b111f911d1f9465555638f0e9fd5
Signed-off-by: Robert Varga <>
6 years agoclustering-test-app: Remove apparently un-used (weird) Tycho dependency 20/51520/3
Michael Vorburger [Tue, 7 Feb 2017 14:56:48 +0000 (15:56 +0100)]
clustering-test-app: Remove apparently un-used (weird) Tycho dependency

I was very surprised to notice something related my old friend Tycho
(who I know well, from a past life..) in c/51447 when glancing over that
change, and wondered what it was used for there, in a particularly
strange fashion... FYI Tycho is, normally, a Maven plugin, used for
among other thing dependency resolution of OSGi bundles from p2
repositories; I've never seen it used as a normal direct dependency, and
can't imagine what it would be good for in normal projects.  So I look
more at this project, and found that Tycho is used for.. nothing at all?

Change-Id: I8863406ea5e629e2fbaef207b0cd4357d6875a7a
Signed-off-by: Michael Vorburger <>
6 years agoDo not scan transitive dependencies 47/51447/6
Robert Varga [Sun, 5 Feb 2017 20:52:08 +0000 (21:52 +0100)]
Do not scan transitive dependencies

Doing so pulls in aries, which is provided by karaf, causing reactivation
of the platform, breaking pax-exam tests. This fixes the issue partially,
as blueprint is not being directly reactivated.

This is true for odl-mdsal-broker-local and odl-mdsal-xsql. Also the sample
test app should not be pulling tycho into an OSGi container, make sure the
dependency is only for test.

Change-Id: I5b08e19e39347fe6e5c03bf6dbe1cb05e36274e3
Signed-off-by: Robert Varga <>
6 years agoRevert "Support multithreading DataBrokerTestCustomizer (AbstractDataBrokerTest)" 67/51467/1
Michael Vorburger [Mon, 6 Feb 2017 12:47:24 +0000 (13:47 +0100)]
Revert "Support multithreading DataBrokerTestCustomizer (AbstractDataBrokerTest)"

This reverts commit 930817d4f1e01c7e08fd7e8ad79bf781cb53034a of Bug 7538.

Change-Id: Ie26f217a2f64cb4eea2d64f2280f6d9773713b21
Signed-off-by: Michael Vorburger <>
6 years agoRemove bad manifestLocation from maven-bundle-plugin 36/51236/2
Michael Vorburger [Tue, 31 Jan 2017 13:15:52 +0000 (14:15 +0100)]
Remove bad manifestLocation from maven-bundle-plugin

Change-Id: I7741c11a21393510af4dcb452e721abd236a9c2c
Signed-off-by: Michael Vorburger <>
6 years agoSupport multithreading DataBrokerTestCustomizer (AbstractDataBrokerTest) 07/51307/5
Michael Vorburger [Wed, 1 Feb 2017 16:03:16 +0000 (17:03 +0100)]
Support multithreading DataBrokerTestCustomizer (AbstractDataBrokerTest) has full background

Bug: 7538
Change-Id: Ic3f2169e18928e3010e45bddb43186cc53d897f4
Signed-off-by: Michael Vorburger <>
6 years agoBUG-7608: activate action-service element 14/51114/9
Robert Varga [Fri, 27 Jan 2017 12:39:53 +0000 (13:39 +0100)]
BUG-7608: activate action-service element

With downstream users fixed up, we can activate the action-service
element to actually require a promise of instantiation of actions.

Change-Id: I3f87acfd713936a4877822b2f62b5a7d2be46107
Signed-off-by: Robert Varga <>
6 years agoAdd Karaf 4 features 61/26161/14
Stephen Kitt [Thu, 19 Jan 2017 14:19:17 +0000 (15:19 +0100)]
Add Karaf 4 features

This moves the Karaf 3 features and adds matching Karaf 4 features for
all the controller features.

Blueprint is upgraded to match the version used in Karaf 4.

Bug: 7526
Change-Id: Ie41cd727942ad2083ef0ca33e03f0d08003c546d
Signed-off-by: Stephen Kitt <>
6 years agoFix an eclipse warning 21/51121/3
Robert Varga [Fri, 27 Jan 2017 13:28:57 +0000 (14:28 +0100)]
Fix an eclipse warning

Eclipse warns about duplicate groupId, as it is inherited
from parent. Fix that by removing it.

Change-Id: I6d99e1d0a547da7077216c83f6d096eb2a2fb5ee
Signed-off-by: Robert Varga <>
6 years agoFix breakage introduced by logback bump 60/51260/1
Robert Varga [Tue, 31 Jan 2017 15:53:45 +0000 (16:53 +0100)]
Fix breakage introduced by logback bump

This is an ugly fix to adjust for changes in logmack 1.1.9.

Change-Id: I424687fe757e5d7cb4525b5954d283f762a0d011
Signed-off-by: Robert Varga <>
6 years agoRemove unneded RoutingTable time tracking 07/50607/11
Robert Varga [Wed, 18 Jan 2017 00:49:13 +0000 (01:49 +0100)]
Remove unneded RoutingTable time tracking

Since we no longer use the time stored in internal entries,
we can use a Set instead of a Map. Adjust internal structure
of RoutingTable and tighten its exported methods.

Change-Id: Ifa2ed53ffc9d05c658927167a4e7dd331c3c2006
Signed-off-by: Robert Varga <>
6 years agoRemove Copier interface 91/50591/11
Robert Varga [Wed, 18 Jan 2017 00:37:28 +0000 (01:37 +0100)]
Remove Copier interface

This is an abstraction which is completely unneeded
at the Bucket level, as BucketData is expected to be immutable
and it is only mutated in RpcRegistry, which has a package-private
relationship with RoutingTable.

Rather than forcing an interface to be implemented and not used,
remove Copier and let RpcRegistry deal with RoutingTable however
seewit sees fit.

Change-Id: Id96a113638d0240c2f0aa9321c5fc4b4e2384a79
Signed-off-by: Robert Varga <>
6 years agoBUG-7573: add BucketStore source monitoring 85/50585/12
Robert Varga [Tue, 17 Jan 2017 23:09:34 +0000 (00:09 +0100)]
BUG-7573: add BucketStore source monitoring

Add BucketData interface capture, which exposes an optional ActorRef.
If this reference is given for a Bucket's data, the bucket will be tied
to the source actor's lifecycle via DeathWatch.

If such an actor is not provided, only basic cluster-level monitoring
will be done.

Change-Id: I794bbf9b360d0c3bf68b29e6869a4f5c7c0d2470
Signed-off-by: Robert Varga <>
6 years agoBUG-3128: cache ActorSelections 84/50584/12
Robert Varga [Tue, 17 Jan 2017 22:58:01 +0000 (23:58 +0100)]
BUG-3128: cache ActorSelections

This is a performance optimization. Since the ActorSelection
for a remote node is an invariant, keep a handy cache of
these objects so we do not have to construct them on every

Change-Id: I820c1d9be5c198a6cac7932b0de0e0776b35b0a5
Signed-off-by: Robert Varga <>
6 years agoBUG-3128: rework sal-remoterpc-connector 88/50488/26
Robert Varga [Fri, 13 Jan 2017 17:30:22 +0000 (18:30 +0100)]
BUG-3128: rework sal-remoterpc-connector

This patch reworks the implementation to take advantage
of the services provided by DOMRpcService, notifying us
of locally-available services.

Previously we have registered all routed RPCs known in
the SchemaContext for global routing context, which has
causes lookups for routed RPCs not otherwise bound to
call back to the remote connector, which then performed
a router lookup.

This approach is slow as each RPC invocation incurs
an additional round-trip to the RpcRegistry to lookup
the appropriate router before the request is sent to it.
It also does not work for global RPCs, because they
only ever have a global context -- hence the routing
decision needs to be made solely in DOMRpcService.

With this patch we maintain a single higher-cost
implementation registered towards each remote node,
handling RPCs discovered via gossiping RoutingTables.
The implementation dispatches requests directly to that
node's RpcInvoker (formerly known as RpcBroker). That
way DOMRpcService will perform internal routing based
on cost and invoke our service only if there are no
local implementations registered.

RpcRegistry no longer performs delayed router discovery
and instead dispatches RoutingTable bucket updates to
a new actor, RpcRegistrator, whose sole job is to maintain
registrations of RemoteRpcImplementation instances for
each remote node with DOMRpcProviderService.

Because of DOMRpcService's ability to filter registration
notifications, our RpcListener will never be notified
of our registrations, which precludes routing loops. We
can therefore remote RemoteRpcInput, whose sole purpose
was to act as a loop detector.

Futher cleanup is done to RpcManager and RemoteRpcProvider
lifecycle, as these now correctly terminate their children
and remove registrations on both restarts and shutdowns.
RpcManager's children startup is also moved from the
constructor to preStart(), so as correctly plug into
the actor lifecycle.

Gossiper is updated to forward node removals to the BucketStore,
so that buckets from unreachable nodes are removed as soon
as possible.

BucketStore is updated to pass down changed buckets
to the subclass whenever a bucket is removed or updated.
It also requires the subclass to provide the initial bucket
data item -- which makes it obvious that a bucket's data
can never be null. This was previously achieved by sending
updating the bucket data from the subclass constructor.

BucketStore/Gossiper messages are updated to be immutable,
which simplifies their instantiation and ensures that they
do not contain nulls (which is required anyway).

Change-Id: I4efb3ddd8ea46ae5be1eb59f1d4fe508f2bc5763
Signed-off-by: Robert Varga <>
6 years agoBUG-7608: Add ActionServiceMetadata and ActionProviderBean 84/50784/18
Robert Varga [Fri, 20 Jan 2017 21:04:11 +0000 (22:04 +0100)]
BUG-7608: Add ActionServiceMetadata and ActionProviderBean

This patch add the new concepts of action-provider and

The implementation does nothing, as we are transitioning from
a run-time logic being coupled with sal-remoterpc-connector.

This allows us to migrate users, while retaining behavior indepent
of sal-remoterpc-connector's actions. This will allow us to fix

Once it is fixed, and DOMRpcRouter can express the action-provider
advertisement, we are going to actiovate the commented-out code

Change-Id: I3f412d092c10b51a198721f288fdefdfc907f0b7
Signed-off-by: Robert Varga <>
6 years agoBUG-7608: Clarify DOMRpc routing/invocation/listener interactions 34/51034/4
Robert Varga [Wed, 25 Jan 2017 21:47:02 +0000 (22:47 +0100)]
BUG-7608: Clarify DOMRpc routing/invocation/listener interactions

As it happens our DOMRpcProviderService is under-documented around
routing behavior and how it interacts with respect to both
DOMRpcService and DOMRpcAvailabilityListener.

Fix this by defining the interactions the same way they are implemented
in the only implementation, DOMRpcRouter.

The fallout of these clarifications is that blueprint's interpretation
of the API contract covers only the RFC6020 RPC part correctly and falls
short of the RFC7950 action case.

This shortcoming will be addressed in a follow-up patch.

Change-Id: I2572c21b7aa6f24b9e2ed37f446b76a032f1880b
Signed-off-by: Robert Varga <>
6 years agoBUG-7697: add defences against nulls 50/51150/2
Robert Varga [Fri, 27 Jan 2017 19:40:49 +0000 (20:40 +0100)]
BUG-7697: add defences against nulls

Null listener is invalid, also make sure we do not ever set
prevRpcs to null.

Change-Id: If8cd16e93a2a07c77a26569c8ecacdc35696cea1
Signed-off-by: Robert Varga <>
6 years agoBUG-6937: Add ReachableMember case to Gossiper 79/51079/3
Tomas Cere [Thu, 26 Jan 2017 17:36:33 +0000 (18:36 +0100)]
BUG-6937: Add ReachableMember case to Gossiper

In case we detect a member down event we remove that member's
address from the whitelist, leading to further gossips being

Subscribe to ReachableMember event to receive notifications
when the cluster heals, so we propagate re-add the member
back to the whitelist.

Change-Id: Id6b366edfa2be89e1a15225d2cad786bbf552129
Signed-off-by: Tomas Cere <>
Signed-off-by: Robert Varga <>
6 years agoDrop explicit dependency on Findbugs annotations 22/50922/2
Stephen Kitt [Tue, 24 Jan 2017 09:05:20 +0000 (10:05 +0100)]
Drop explicit dependency on Findbugs annotations

odlparent provides jsr305, and will eventually provide annotations
instead; so the dependencies no longer need to be explicitly added to
downstream projects.

Bug: 7663
Change-Id: If8fa8b231f9cb3d4a5e4fbdbd1b58e720756f5c0
Signed-off-by: Stephen Kitt <>
6 years agoMake AbstractDOMBroker.isSupported() static 79/50379/4
Robert Varga [Thu, 12 Jan 2017 17:45:39 +0000 (18:45 +0100)]
Make AbstractDOMBroker.isSupported() static

This is a utility method, make it static and simplify
it it.

Change-Id: I90e4d8b54b92fd319f373b4ecd0137fc8c5575ef
Signed-off-by: Robert Varga <>
6 years agoUse opendaylight-karaf-empty from Odlparent 49/50549/2
Vratko Polak [Tue, 17 Jan 2017 14:14:48 +0000 (15:14 +0100)]
Use opendaylight-karaf-empty from Odlparent

The Controller one is kept for project not migrated to features-parent yet.

Change-Id: I2354a5e738449494486fc8e433afdc302c6bb7f8
Signed-off-by: Vratko Polak <>
6 years agoBUG-3128: do not open-code routed RPC identification 34/50734/4
Robert Varga [Fri, 20 Jan 2017 12:15:04 +0000 (13:15 +0100)]
BUG-3128: do not open-code routed RPC identification

Identification of routed RPCs is available via RpcRoutingStrategy.
Use that code to identify routed RPC instead of duplicating same
logic in multiple places.

Change-Id: I5a12b8fd891cb41f805b2a4e7ae465d4004aca39
Signed-off-by: Robert Varga <>
6 years agoReplace mockito-all by mockito-core (see Bug 7662) 61/50861/3
Michael Vorburger [Mon, 23 Jan 2017 18:58:42 +0000 (19:58 +0100)]
Replace mockito-all by mockito-core (see Bug 7662)

The change req. to includeArtifactIds is kinda similar to

Funky - this was the ONLY one of almost 40 changes
made in bulk re. Bug 7662 which required additional
manual analysis and intervention... ;-)

Bug: 7662
Change-Id: Ibd14dd061024056392a020364647a70a87f2461d
Signed-off-by: Michael Vorburger <>
6 years agoReplace FindBugs :jsr305 by full :annotation (Bug 7663) 98/50898/2
Michael Vorburger [Mon, 23 Jan 2017 20:04:29 +0000 (21:04 +0100)]
Replace FindBugs :jsr305 by full :annotation (Bug 7663)

Change-Id: I76584c682c1e5d5acdbdb3b420f5a56da0aa363e
Signed-off-by: Michael Vorburger <>
6 years agoRemove DOMRpcIdentifier.GLOBAL_CONTEXT 27/50627/2
Robert Varga [Wed, 18 Jan 2017 16:30:35 +0000 (17:30 +0100)]
Remove DOMRpcIdentifier.GLOBAL_CONTEXT

This is a shorthand for YangInstanceIdentifier.EMPTY, hence
we can inline the definition.

Change-Id: Icb8f025feb48cbc6add7c30d1db863b19c18f546
Signed-off-by: Robert Varga <>
6 years agoAdd singleton service based global rpc 24/50724/3
Tomas Cere [Fri, 20 Jan 2017 15:52:53 +0000 (16:52 +0100)]
Add singleton service based global rpc

We need to have a global rpc present that we can use in csit,
use ClusterSingletonService to have it registered only on one
node in cluster.

Change-Id: Id4019ebb874a1da7919d1484ec7af7f5b3395283
Signed-off-by: Tomas Cere <>
6 years agoBUG-7594: Expand NormalizedNodeData{Input,Output} to handle SchemaPath 56/50656/6
Robert Varga [Thu, 19 Jan 2017 10:26:11 +0000 (11:26 +0100)]
BUG-7594: Expand NormalizedNodeData{Input,Output} to handle SchemaPath

These utility classes are already dealing with QNames, so it makes
sense to expand their capabilities to include SchemaPath serialization.

Change-Id: Ibcb931f78959eb57f834cd2892511c4963638caa
Signed-off-by: Robert Varga <>
6 years agoBug 5280: Add ProgressTracker 53/49053/18
Vratko Polak [Thu, 22 Dec 2016 09:39:00 +0000 (10:39 +0100)]
Bug 5280: Add ProgressTracker

ProgressTracker determines the tracking delay required
to keep the outbound queue reasonably full. Unlike Guava's
RateLimiter, we can assume cooperative callers, hence we
can use an enqueue-and-wait strategy as appropriate. This
lowers contention in multi-threaded access, as the wait
part of the cycle is spent outside the lock.

Change-Id: Iaea65c171455b89d7117431599ebc65fe0a4f19a
Signed-off-by: Vratko Polak <>
Signed-off-by: Robert Varga <>
6 years agoBUG-6937: correct format string 74/50574/2
Robert Varga [Tue, 17 Jan 2017 20:46:18 +0000 (21:46 +0100)]
BUG-6937: correct format string

This is a fix, the constructor takes String.format() string,
not a Logger.debug() one.

Change-Id: I966b02cd0c280b50ec1d0e77fb5a493fa2f5a4fe
Signed-off-by: Robert Varga <>
6 years agoFix eclipse warnings 33/50633/2
Robert Varga [Wed, 18 Jan 2017 17:14:56 +0000 (18:14 +0100)]
Fix eclipse warnings

Remove illegal @return for method with no return and untangle
if/else block where if branch always returns.

Change-Id: Iafe4eb1b016674375d0b25cd01fdc05068fa1196
Signed-off-by: Robert Varga <>
6 years agoFix FindBugs warning around locking 32/50632/2
Robert Varga [Wed, 18 Jan 2017 17:12:57 +0000 (18:12 +0100)]
Fix FindBugs warning around locking

Correct use of locks is to use a try/finally block, so correct
the usage to shut FB up.

Change-Id: I8e42425a9566f506682a8f3e2c25ac3dfe4ec812
Signed-off-by: Robert Varga <>
6 years agoBUG-7506: use common DocumentBuilderFactory 07/50707/2
Robert Varga [Fri, 20 Jan 2017 10:51:18 +0000 (11:51 +0100)]
BUG-7506: use common DocumentBuilderFactory

Yangtools exports UntrustedXML utility class, which contains pre-configured
document builder factories for dealing with XMLs which are not completely
trusted. Reuse that instead of rolling our own, especially in the XML parse

Change-Id: I83d0ea60104f2266669493548e2d40b5ab6e4772
Signed-off-by: Robert Varga <>
6 years agoBUG-7608: OpendaylightNamespaceHandler methods can be static 06/50706/1
Robert Varga [Fri, 20 Jan 2017 10:46:10 +0000 (11:46 +0100)]
BUG-7608: OpendaylightNamespaceHandler methods can be static

Eclipse emits warnings about methods being potentially static,
clean that up.

Change-Id: I4b5fb6d12486ea20d4a47429eafebe3f8b559c40
Signed-off-by: Robert Varga <>
6 years agoBUG-7608: restructure exception throws 05/50705/1
Robert Varga [Fri, 20 Jan 2017 10:45:18 +0000 (11:45 +0100)]
BUG-7608: restructure exception throws

This cleans up the try-catch block so that we do not have
to re-throw exceptions.

Change-Id: I8c876f12b7a9e9108eb8dcca7a602927a78bec2c
Signed-off-by: Robert Varga <>
6 years agoUpdate dependendency desc properly in RpcServiceMetadata 82/50682/1
Tom Pantelis [Thu, 19 Jan 2017 17:02:57 +0000 (12:02 -0500)]
Update dependendency desc properly in RpcServiceMetadata

When it transitions to waiting for available DOM RPCs, the
dependendency desc is now updated appropriately to reflect
the correct context.

Change-Id: Iaf7108dd664c9ed78444b0f3dfa4e14be431f35f
Signed-off-by: Tom Pantelis <>
6 years agoBUG-3128: Update RPC router concepts 87/50487/6
Robert Varga [Mon, 16 Jan 2017 09:15:49 +0000 (10:15 +0100)]
BUG-3128: Update RPC router concepts

Enrich DOMRpcImplementation with invocation cost, which is taken into account
when deciding which implementation to invoke. This allows local RPCs to be
prioritized over remote ones.

Also add the ability to filter implementations when notifying availability
listener. This allows remote RPC to filter its own registrations, preventing
re-forwarding loops, where a remote implementation would be forwarded as
a local one.

Change-Id: Id1d78d5031904e19134c103e12b79d68cf0b98c3
Signed-off-by: Robert Varga <>
6 years agoCleanup RemoteDOMRpcFuture 58/50558/2
Robert Varga [Tue, 17 Jan 2017 14:56:22 +0000 (15:56 +0100)]
Cleanup RemoteDOMRpcFuture

Add a missing space and make mapException() static.

Change-Id: I4e9c33899bff7e0488dbe8537f4e832e50a3c53e
Signed-off-by: Robert Varga <>
6 years agoHandle UNMODIFIED type in DataTreeCandidateInputOutput 35/50535/1
Tom Pantelis [Tue, 17 Jan 2017 09:23:02 +0000 (04:23 -0500)]
Handle UNMODIFIED type in DataTreeCandidateInputOutput

We now replicate DataTreeCandidates with root node type UNMODIFIED
however the deserialization in DataTreeCandidateInputOutput throws
an ISE with "Unhandled node type 2". The writeDataTreeCandidate
method writes the UNMODIFIED type but readDataTreeCandidate needs
to handle that case as well.

In addition, writeDataTreeCandidate also handles APPEARED and
DISAPPEARED but readDataTreeCandidate does not so I made that
change as well.

Change-Id: I19932e545bbeb95fa10d5f57d43a5780af586285
Signed-off-by: Tom Pantelis <>
6 years agoUse new Karaf 3 odlparent features 92/50092/2
Stephen Kitt [Fri, 6 Jan 2017 17:28:50 +0000 (18:28 +0100)]
Use new Karaf 3 odlparent features

This migrates controller to the new odlparent features. This is useful
as a stable base before the Karaf 4 migration (which only has the new
odlparent features).

Change-Id: Id638e57cdc4f30e2790f5677471310de666b7378
Signed-off-by: Stephen Kitt <>
6 years agoBUG-7446: remove use of deprecated Guava methods 75/50175/2
Robert Varga [Tue, 10 Jan 2017 12:22:55 +0000 (13:22 +0100)]
BUG-7446: remove use of deprecated Guava methods

These methods have been deprecated in 18.0 and removed
in 21.0 (or sooner). Remove their use as a preparatory
patch for migration.

Change-Id: Id905760eb7bf817e475aeb1860f9dba39ce1a77d
Signed-off-by: Robert Varga <>
6 years agoRevert "Fix test failure in DistributedShardedDOMDataTreeTest" 03/50503/2
Tomas Cere [Mon, 16 Jan 2017 13:35:01 +0000 (14:35 +0100)]
Revert "Fix test failure in DistributedShardedDOMDataTreeTest"

This reverts commit d7ed1e572bd9af64fab1e9048b4b27ebeb564f7e.

Also ignore this test for now since distributed-data is broken in the
new akka version. Will be unignored once the distributed-data parts
are reimplemented.

Change-Id: Icb74eeaddedf50a7217806e93d2a17430463381a
Signed-off-by: Tomas Cere <>
6 years agoBUG-5222: do not use reflection on Optional 26/50426/2
Robert Varga [Fri, 13 Jan 2017 13:02:34 +0000 (14:02 +0100)]
BUG-5222: do not use reflection on Optional

Remove the horrible use of reflection to examine Optional
returned from the data store. Also close the transaction

Change-Id: I8141263588552d0768b2d23ec58e157f723e3bf3
Signed-off-by: Robert Varga <>
6 years agoBUG-5222: offload XSQLBluePrint creation to first access 25/50425/2
Robert Varga [Fri, 13 Jan 2017 12:57:34 +0000 (13:57 +0100)]
BUG-5222: offload XSQLBluePrint creation to first access

Constructing XSQLBluePrint in onGlobalContextUpdated() slows
down startup and is utterly inefficient (like all of XSQL).

As a stop-gap measure move its instantiation to first use,
when it is constructed from saved SchemaContext reference.

Also remove uneeded elements field, as it is not used anywhere
and just gets in the way.

Change-Id: I954d2217da6ec8b12d0b980d864cf3d776df78cc
Signed-off-by: Robert Varga <>
6 years agoBug 7518: Temporarily revert the main akka.conf to netty 20/50220/3
Vratko Polak [Wed, 11 Jan 2017 13:28:25 +0000 (14:28 +0100)]
Bug 7518: Temporarily revert the main akka.conf to netty

Change-Id: If31c4fc330917569b5008f2058545a8f0857a5f8
Signed-off-by: Vratko Polak <>
6 years agoFixup Typos in Comments 36/49836/2
Shrenik [Tue, 27 Dec 2016 18:23:55 +0000 (23:53 +0530)]
Fixup Typos in Comments

- Fix typos
- Fix duplicate words

Change-Id: I33eb78183fbc56e4d7c574e486b7f811dd846b18
Signed-off-by: Shrenik Jain <>
6 years agoBug 7469: Advertise CDS DOMDataTreeCommitCohortRegistry 90/50090/1
Tom Pantelis [Fri, 6 Jan 2017 15:59:06 +0000 (10:59 -0500)]
Bug 7469: Advertise CDS DOMDataTreeCommitCohortRegistry

Advertised the CDS DOMDataTreeCommitCohortRegistry implementation
via the DOMDataBrokerExtension mechanism.

I needed to add a DOMDataTreeCommitCohortRegistry interface version
in the controller DOM API package that extends the controller
DOMDataBrokerExtension since the mdsal DOMDataTreeCommitCohortRegistry
version extends the mdsal DOMDataBrokerExtension.

Change-Id: I71daac1cd7c231d071c376206d85786c333bac68
Signed-off-by: Tom Pantelis <>
6 years agoBug 7362: Notify applyState synchronously 04/49404/7
Tom Pantelis [Thu, 15 Dec 2016 06:05:53 +0000 (01:05 -0500)]
Bug 7362: Notify applyState synchronously

As per comments in bug 7362, it's problematic for snapshots that we
set lastApplied index but queue the ApplyState message. This can leave
the derived actor's captured state out-of-sync with the lastApplied index
if a snapshot occurs in between. We need to set lastApplied atomically
with apply to state so I added a callback that RaftActor sets in the
RaftActorContext that is called by the behavior's applyLogToStateMachine
method. The callback immediately calls applyState. The ApplyState message
is still queued for other compenents that trigger on it (and also unit

I considered setting lastApplied when the ApplyState message is processed,
and not in applyLogToStateMachine, but this would be problematic if
another AppendEntriesReply was queued before the ApplyState message
as it would try to apply it again.

Change-Id: Ie062af8440bc251eec9c9ef58f450dee23abd04c
Signed-off-by: Tom Pantelis <>
6 years agoFix test failure in DistributedShardedDOMDataTreeTest 76/50076/1
Tom Pantelis [Thu, 5 Jan 2017 20:26:57 +0000 (15:26 -0500)]
Fix test failure in DistributedShardedDOMDataTreeTest

Introduced by the akka artery patch
( which wasn't
rebased after
was merged.

Change-Id: I1b5c8d7124d89eb90c74290f4571c0db1d2fb4ac
Signed-off-by: Tom Pantelis <>
6 years agoUse Akka artery for remote transport 66/49466/10
Robert Varga [Fri, 16 Dec 2016 10:23:28 +0000 (11:23 +0100)]
Use Akka artery for remote transport

This transport is introduced in 2.4.11 and is supposed to be
faster than the TCP transport. Enabled it globally so we can
evaluate it.

Change-Id: I25234f82ac056700e8b56abaeb452c53ec5b9dbd
Signed-off-by: Robert Varga <>
Signed-off-by: Tom Pantelis <>
6 years agoBUG-7159: eliminate use of CrossStatementSourceReactor 47/49747/5
Robert Varga [Thu, 22 Dec 2016 15:27:26 +0000 (16:27 +0100)]
BUG-7159: eliminate use of CrossStatementSourceReactor

yang-test-util provides a convenient way to parse YANG files
into a SchemaContext, without relying on the internal layout
of the yang parser. Convert tests to use these utilities,
which simplifies test setup and allows the yang parser layout
to evolve without breaking the world.

Change-Id: Icbd0556b990ea9d5ff93c34330049a9683e04970
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: Fix deadlock with TransmitQueue 24/50024/4
Robert Varga [Wed, 4 Jan 2017 15:10:57 +0000 (16:10 +0100)]
BUG-5280: Fix deadlock with TransmitQueue

TransmitQueue should not be dispatching responses while
the connection lock is being held, as a that may expose
clients to AB/BA deadlocks if the callback tries to acquire
a lock which is being held by another thread attempting
to touch the same connection (i.e. transmit).

Fix this by TransmitQueue returning the entry to be
completed and AbstractClientConnection checking its
presence and performing actual completion after it has
dropped the connection lock.

Change-Id: Ibbacb641a297bfe7d4790af8401b036285c26593
Signed-off-by: Robert Varga <>
6 years agoBUG-7033: Remove payload replication short-circuits 71/49971/2
Tom Pantelis [Tue, 3 Jan 2017 12:53:08 +0000 (07:53 -0500)]
BUG-7033: Remove payload replication short-circuits

Removed the code paths in ShardDataTree#startCommit whereby it
short-circuits the call to persistPayload. This simplifies the
code in ShardDataTree. For UNMODIFIED DataTreeCandidates we still
want to replicate so followers have an accurate view of transaction
states. For the case where persistence is disabled and there are no
followers, I modified persistPayload to check those conditions and

Change-Id: Ifb940f950b076b5f9cedd91c42d3e065db92a7c5
Signed-off-by: Tom Pantelis <>
6 years agoRev the archetype to utilize the latest year for default copyright value 86/49986/1
Ryan Goulding [Tue, 3 Jan 2017 18:18:41 +0000 (13:18 -0500)]
Rev the archetype to utilize the latest year for default copyright value

The archetype still defaults copyright year to 2016.  This revs the year
to 2017.

Change-Id: I9c51d3df71c4a79331c7152ed846923d60727a69
Signed-off-by: Ryan Goulding <>
6 years agoBUG 2138: Introduce DistributedShardedDOMDataTree 43/44943/61
Tomas Cere [Wed, 31 Aug 2016 15:22:30 +0000 (17:22 +0200)]
BUG 2138: Introduce DistributedShardedDOMDataTree

This is the initial patch that introduces the concept of
prefix based shards and consumer/producer api's into the

Each shard has a frontend registered into the ShardedDOMDataTree
that forwards calls into the DistributedDataStoreClient frontend
which then handles the actual writes into the Shard.
These ShardFrontends are then distributed into other nodes
via ShardedDataTreeActor which also handles notification of
other nodes of open Producers.

Change-Id: Ifcbd1021fdaeac7929fc547e6e32be50da9d93fc
Signed-off-by: Tomas Cere <>
Signed-off-by: Robert Varga <>
6 years agoBUG-7033: Fix commit exception due to pipe-lining 62/49762/3
Tom Pantelis [Thu, 22 Dec 2016 21:57:22 +0000 (16:57 -0500)]
BUG-7033: Fix commit exception due to pipe-lining

The DistributedDataStoreRemotingIntegrationTest#testTransactionWithIsolatedLeader
has failed sporadically on commit with the "Store tree X and candidate
base Y differ" error due to an edge case bug with the pipe-lining. Basically
this occurs if tx 1 is pending completion of replication and tx 2 is progressed
to the COMMIT_PENDING state but the associated DataTreeCandidate has
ModificationType.UNMODIFIED. In that case we elide replication and proceed
immdiately to finishCommit which results in the error due to tx 2 committing
before tx 1. To fix it, I added a new FINISH_COMMIT_PENDING state and call
payloadReplicationComplete, which checks the head of the pendingFinishCommits
queue, when replication is elided..

Change-Id: I5a0d033df131c9c3f4e24670a02a971dec331a4d
Signed-off-by: Tom Pantelis <>
6 years agoBUG-7033: Follow-up to address prior comments 89/49689/2
Tom Pantelis [Wed, 21 Dec 2016 10:51:39 +0000 (05:51 -0500)]
BUG-7033: Follow-up to address prior comments

Follow-up patch to address the 2 comments in that were made
after merge..

Change-Id: Id9864d89637e72961fc1a259f25d541edb1b46dc
Signed-off-by: Tom Pantelis <>
6 years agoBUG-7033: Implement batchHint in ShardDataTree 98/49498/4
Tom Pantelis [Fri, 16 Dec 2016 21:05:15 +0000 (16:05 -0500)]
BUG-7033: Implement batchHint in ShardDataTree

Modified ShardDataTree to properly compute the batchHint flag
that is passed to persistData. Basically if the next transaction
in the pendingCommits queue is in the COMMIT_PENDING state then
batchHint is set to true. After the call to persistData the next
transaction in the pendingCommits queue is processed which, if
COMMIT_PENDING, will be batched for replication with the previous

Change-Id: I938e98391c69d617901e7e179c93e066667c017d
Signed-off-by: Tom Pantelis <>
6 years agoBug 7391: Fix out-of-order LeaderStateChange events 01/49701/3
Tom Pantelis [Wed, 21 Dec 2016 14:20:36 +0000 (09:20 -0500)]
Bug 7391: Fix out-of-order LeaderStateChange events

On leader transition, the current leader first sends out
LeaderTransitioning events to each follower to tell them the
current leader is being deposed. The followers send out a
LeaderStateChange event with a null leaderId which is picked
up by the ShardManager to delay subsequent transactions activity
to the shard until a new leader is elected. However it's possible
the LeaderStateChange message does not reach a follower until after
the leader transition occurs (eg due to dispatching delay in the
caller or the network). This results in a LeaderStateChange event
with a null leaderId being delivered after the LeaderStateChange
with the new leaderId. I wrote a unit test that reproduces it.

We need to handle LeaderTransitioning events in a CAS-like manner,
ie include the leaderId with the LeaderTransitioning message and
only issue the LeaderStateChange event with a null leaderId if the
current leaderId matches the leaderId in the LeaderTransitioning

Change-Id: I24e8bbf7707858ac4ed62f3a979cc0403daff8ac
Signed-off-by: Tom Pantelis <>
6 years agoBUG-5280: fix race proxy creation and reconnect 05/49705/2
Robert Varga [Wed, 21 Dec 2016 15:11:09 +0000 (16:11 +0100)]
BUG-5280: fix race proxy creation and reconnect

ProxyHistory map insertions have to happen-before or
happen-after the reconnect attempt. This is mostly take care
of by client's inversible lock, except for the race window
when the connection lookup succeeds and the history map
is updated.

Resolve this by introducing a StampedLock, which is used
to establish order of these operations, covering the entire
createHistoryProxy() method.

Change-Id: Ibdee739c94f3a48bef3f7fb19cd2f041c0061fe2
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: fix InversibleLock race 98/49698/1
Robert Varga [Wed, 21 Dec 2016 13:17:35 +0000 (14:17 +0100)]
BUG-5280: fix InversibleLock race

Do not read latch twice, as it may end up being
cleared asynchronously. This is a typo, as the reuse
is exactly why we read the latch into a local variable.

Change-Id: If90bd157be6e20d69f158bc749f163c1008541a4
Signed-off-by: Robert Varga <>
6 years agoBUG-7033: Add batchHint flag to RaftActor#persistData 39/49439/5
Tom Pantelis [Thu, 15 Dec 2016 15:28:07 +0000 (10:28 -0500)]
BUG-7033: Add batchHint flag to RaftActor#persistData

Added a batchHint flag where, if true, it elides sending AppendEntries
immediately. AppendEntries will be sent on the next persistData call
with batchHint false or at the next heartbeat interval.

Change-Id: Id887ab46d5147e4fb5e50e52acaa8887d4048e1d
Signed-off-by: Tom Pantelis <>
6 years agoBUG-7033: Implement pipe-lining in ShardDataTree 84/49384/6
Tom Pantelis [Wed, 14 Dec 2016 19:41:23 +0000 (14:41 -0500)]
BUG-7033: Implement pipe-lining in ShardDataTree

Change-Id: I54d6e741089072660a9ea6df20801b9a196e7b52
Signed-off-by: Tom Pantelis <>
6 years agoBUG-2138: Allow creation of prefixed ShardDataTrees 18/48218/53
Tomas Cere [Thu, 10 Nov 2016 13:41:30 +0000 (14:41 +0100)]
BUG-2138: Allow creation of prefixed ShardDataTrees

This actually starts using the storeRoot field from DataStoreContext
so we can have CDS shards rooted somewhere else than store root.

Change-Id: I304a7678f3077359e7b2c4007e73321544c6e798
Signed-off-by: Tomas Cere <>
6 years agoFixup eclipse warnings in ClusterAdminRpcServiceTest 56/49556/1
Robert Varga [Mon, 19 Dec 2016 14:08:55 +0000 (15:08 +0100)]
Fixup eclipse warnings in ClusterAdminRpcServiceTest

- boxing of true/false
- potentially static private method

Change-Id: I34612b20f57c8463de3a02744e271fa2dd774728
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: split DistributedDataStore 07/48707/48
Robert Varga [Fri, 25 Nov 2016 12:55:13 +0000 (13:55 +0100)]
BUG-5280: split DistributedDataStore

Split the DistributedDataStore into two components into
an abstract base class and concretization running with
TransactionProxies. Add another concretization, which uses
DataStoreClient to instantiate requests.

Change-Id: I454eec76d54c2fd4e4ea1e5cd16d12398eec81f0
Signed-off-by: Robert Varga <>
6 years agoFix JMX generator unit test 42/49442/2
Robert Varga [Thu, 15 Dec 2016 16:41:31 +0000 (17:41 +0100)]
Fix JMX generator unit test

MDSAL change to use static factory methods for
instantiating well-known types has changed the way
Longs are instantiated, breaking this test. Fix it up
to reflect the change.

Change-Id: I930c04baf07005216d81cfe1e123b10b21cdc396
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: speed FrontendClientMetadataBuilder up 64/49264/6
Robert Varga [Mon, 12 Dec 2016 18:07:43 +0000 (19:07 +0100)]
BUG-5280: speed FrontendClientMetadataBuilder up

ensureHistory() boils down to a simple computeIfAbsent(),
so make it more concise by using a simple constructor lambda.

Change-Id: Ia093ced071ae5e0411633d3c4f501a4e94c5de43
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: synchronize access to local histories 27/49427/5
Robert Varga [Thu, 15 Dec 2016 13:24:47 +0000 (14:24 +0100)]
BUG-5280: synchronize access to local histories

There is a subtle race between the allocation of histories
and the reconnect process, which could allow a local history
to be created when its connection is being reestablished but
after the cohorts for the connection have already been captured.

Change-Id: I230b5c00844d8e82775efc8f70368c2f63eabb1e
Signed-off-by: Robert Varga <>
6 years agoCleanup equals() template 45/49445/2
Robert Varga [Thu, 15 Dec 2016 17:22:00 +0000 (18:22 +0100)]
Cleanup equals() template

Missing a space in 'if(' and has explicit comparison to false,
which is not needed. Clean that up.

Change-Id: I55f748935070378eab58dd11cf2fb91e8cc9628b
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: add ClientTransactionCursor 24/49424/3
Robert Varga [Thu, 15 Dec 2016 11:25:59 +0000 (12:25 +0100)]
BUG-5280: add ClientTransactionCursor

Cursor-based access is useful for new MD-SAL APIs,
expose it from ClientTransaction.

Change-Id: Iff57c9d09935181e02ab5ac3f42c9f70d9f95424
Signed-off-by: Robert Varga <>
6 years agoBUG-7033: Allow ShardDataTree to pipeline transactions 75/28775/21
Robert Varga [Sat, 24 Oct 2015 16:34:53 +0000 (18:34 +0200)]
BUG-7033: Allow ShardDataTree to pipeline transactions

InMemoryDataTree gives us the DataTreeTip, which allows another
DataTreeModificatoin to be prepared on top an uncommitted one. This
allows us to pipeline transactions if we manage our lifecycle properly.

For now this feature gives stricter abort validation, and has otherwise
no impact. In future, this allows another DataTreeModification to be
prepared and queue for persistence before replication of the previous
one finishes.

Change-Id: I7ab97c906a6403da780800edd335f74c403e5aa4
Signed-off-by: Robert Varga <>
Signed-off-by: Tom Pantelis <>
6 years agoRemove RV_RETURN_VALUE_IGNORED FB supression 59/49259/3
Robert Varga [Mon, 12 Dec 2016 17:36:31 +0000 (18:36 +0100)]

Turning poll() into remove() makes the warning go away.
We are also switching the cases where this is not reported
to maintain consistency and force a different error than
NPE on inconsistency (which should never happen anyway).

Change-Id: I6b7739a2e5bcd00b7745af87b9c5e180006400bd
Signed-off-by: Robert Varga <>
6 years agofix typo sotred to sorted 01/49101/2
Jamo Luhrsen [Wed, 7 Dec 2016 18:30:13 +0000 (10:30 -0800)]
fix typo sotred to sorted

Change-Id: If1d781dd961b6ddea94cb070b7ad1a9aac37cb95
Signed-off-by: Jamo Luhrsen <>
6 years agochaging trivial log from info to warn 00/49100/2
Jamo Luhrsen [Wed, 7 Dec 2016 18:13:01 +0000 (10:13 -0800)]
chaging trivial log from info to warn

this log message is given a lot in the dsbenchmark test and doesn't really
help, as far as I can tell.  So, I changed it to warn and hopefully made it
a little more descriptive.

Change-Id: I784f5ca12eebd531b541868b2af9b6aff08a0737
Signed-off-by: Jamo Luhrsen <>
6 years agoBug 7271: Use mdsal AbstractDOMStoreTreeChangePublisher in CDS 03/48903/4
Tom Pantelis [Thu, 1 Dec 2016 18:32:29 +0000 (13:32 -0500)]
Bug 7271: Use mdsal AbstractDOMStoreTreeChangePublisher in CDS

Modified DefaultShardDataTreeChangeListenerPublisher to derive from
the AbstractDOMStoreTreeChangePublisher in the mdsal project to get
the DataTreeCandidate batching.

Change-Id: Ic86da04a80e9db56dd234549b88f4c958b1a708c
Signed-off-by: Tom Pantelis <>
6 years agoBUG-5280: add READY protocol 50/49250/5
Robert Varga [Mon, 12 Dec 2016 11:43:31 +0000 (12:43 +0100)]
BUG-5280: add READY protocol

In order to make chained transactions work with remoting
we need a way for the frontend to propagate the ready state
to the backend. This patch adds a READY protocol, which acts
as a preparatory stage before the 'real' protocol kicks in.

With that the backend state is properly updated to reflect
state transitions on the frontend and we do not need to play
weird future-based delays in the frontend, which would be
exceedingly complex.

Change-Id: I51a0ca0c2b900e3c6522426e5897a4fca1b9da19
Signed-off-by: Robert Varga <>
6 years agoBUG 2138: Introduce prefix based shards into ShardManager 05/44705/48
Tomas Cere [Wed, 31 Aug 2016 15:20:38 +0000 (17:20 +0200)]
BUG 2138: Introduce prefix based shards into ShardManager

Adds the concept of shards rooted at a DOMDataTreeIdentifier
(combination of YangInstanceIdentifier and LogicalDataStore)
into the distributed datastore.

Change-Id: I43a32556000092c7e7b2ee09b334f82f38ec865b
Signed-off-by: Tomas Cere <>
6 years agoBUG-5280: add basic concept of ClientSnapshot 27/48727/32
Robert Varga [Fri, 25 Nov 2016 12:55:13 +0000 (13:55 +0100)]
BUG-5280: add basic concept of ClientSnapshot

In order to accurately read-only transactions with ClientLocalHistory,
we need to differentiate between Transactions and Snapshots. This patch
introduces the concept, its API and backend signalling/implementation.

State keeping is reworked so it requires only a single field, which
is manipulated via an atonic updater, with null signifying state has
already been closed (or is in process of being taken care of).

Change-Id: I2f8fd5ffdff366d1948538299b96721b756c620c
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: expose queue messages during reconnect 80/48980/11
Robert Varga [Thu, 1 Dec 2016 15:28:53 +0000 (16:28 +0100)]
BUG-5280: expose queue messages during reconnect

This patch reworks the internals of AbstractClientConnection
to isolate the TransmitQueue from the rest of the logic,
so we have proper split between implementation and interface
exposed to the users.

Furthermore the public interface is slightly reworked so the
individual Proxies have access to the (locked) queue contents,
which is needed to correctly replay transaction state within
transaction chains.

Change-Id: I1c08fa06eec4dd581e07002059c5142e6b0c1ed4
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: report history which was not found 98/48998/7
Robert Varga [Mon, 5 Dec 2016 17:35:54 +0000 (18:35 +0100)]
BUG-5280: report history which was not found

We seem to have a lockign issue somewhere, which leads
to an exception being thrown in tests. Refactor the exception
to also include history identifier, so we can track it down.

Change-Id: I0826dfdc18f10103da58855bb14c269734ae47ab
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: do not cache modify responses 79/48979/5
Robert Varga [Mon, 5 Dec 2016 10:56:04 +0000 (11:56 +0100)]
BUG-5280: do not cache modify responses

Modification responses cannot be cached, as their sequence
number is changing -- which leads to cached responses being

Change-Id: I3c4037e3a29cff3cdd193865cd98f5df152286f4
Signed-off-by: Robert Varga <>
6 years agoUse nullOrNonPositive instead of nullOrNonZero 81/48981/2
Jacky Hu [Mon, 5 Dec 2016 12:54:26 +0000 (20:54 +0800)]
Use nullOrNonPositive instead of nullOrNonZero

The method name should match it's functionality to test whether the
duration is positive or not.

Change-Id: I830d1072d5854d88a96f02a3f161456329a50be7
Signed-off-by: Jacky Hu <>
6 years agoBug 7326: Fix ConcurrentModificationException in Blueprint 54/49154/4
Tom Pantelis [Fri, 9 Dec 2016 15:35:51 +0000 (10:35 -0500)]
Bug 7326: Fix ConcurrentModificationException in Blueprint

in AbstractDependentComponentFactoryMetadata.stopServiceRecipes()

tpantelis: "This is an edge case where the container is destroyed
immediately after and while it's starting up. This isn't likely to
happen in production but can happen during feature tests. I had assumed
the container would provide the protection but apparently it doesn't."

Change-Id: Id7532d30cb0a5f67fd907cb15372069d8769b247
Signed-off-by: Michael Vorburger <>
Signed-off-by: Tom Pantelis <>
6 years agoRemove FB suppression 39/49139/1
Robert Varga [Thu, 8 Dec 2016 09:34:26 +0000 (10:34 +0100)]
Remove FB suppression

Using .remove() instead of .poll() makes FB  shut up,
because the return value is not used for signalling
queue state.

Change-Id: I0aefc0eb7ede948b8311d12c6307137532018386
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: refactor AbstractClientConnection 64/48864/3
Robert Varga [Thu, 1 Dec 2016 10:17:53 +0000 (11:17 +0100)]
BUG-5280: refactor AbstractClientConnection

The structure of AbstractClientConnection and its subclasses
makes it hard to replay messages in a coordinated fashion. Furthermore
splitting the inflight and pending and inflight queues into separate
classes means we would have to jump through quite a few hoops
to correctly calculate backpressure.

Refactor the base class so it includes all the operations usually
performed, with remoteMaxMessages() acting as the limiter, which disables
transmission in connecting/reconnecting states.

Change-Id: If743e4913aade7ed65ba60375d8b7d12c563cb96
Signed-off-by: Robert Varga <>
6 years agoRemove MockReplicatedLogEntry 08/48808/3
Tom Pantelis [Tue, 29 Nov 2016 20:21:03 +0000 (15:21 -0500)]
Remove MockReplicatedLogEntry

This class now just inherits from SimpleReplicatedLogEntry so in no
longer needed. Changed all users to SimpleReplicatedLogEntry. The only
difference is that the term and index params are flipped in the ctors.

Change-Id: I0b32078eeb9ea45001dcd8e8aa30bfe548256e96
Signed-off-by: Tom Pantelis <>
6 years agoChange ReplicatedLogImplEntry to Externalizable proxy pattern 03/48803/2
Tom Pantelis [Tue, 29 Nov 2016 16:34:50 +0000 (11:34 -0500)]
Change ReplicatedLogImplEntry to Externalizable proxy pattern

The Externalizable proxy pattern decreases serialized sized as
it avoids serializing the full class info. We need to keep
ReplicatedLogImplEntry for persistence backwards compatibility.
A new SimpleReplicatedLogEntry class was added to implement the
Externalizable proxy pattern. ReplicatedLogImplEntry "readResolves"
to SimpleReplicatedLogEntry. Also, SimpleReplicatedLogEntry
implements MigratedSerializable to cause a snapshot on upgrade
enabling us to remove ReplicatedLogImplEntry in the next release.

Change-Id: Iaf5c8f6b271c3ead43a80ac905412b4cb5a9efa2
Signed-off-by: Tom Pantelis <>
6 years agoBUG-5280: fix transaction seal atomicity 31/48831/2
Robert Varga [Wed, 30 Nov 2016 14:07:58 +0000 (15:07 +0100)]
BUG-5280: fix transaction seal atomicity

AbstractProxyTransaction.seal() indicates that the user is done
with the transaction. This transition needs to be atomically
propagated to successors on reconnect, such that the user will
always observe sealed proxies. More importantly this state
is propagated to parent ProxyHistory, where it drives the state
machine in ClientProxyHistory -- and failing to mark the successor
as sealed will wreck that.

Unfortunately an AbstractProxyTransaction does not forward all
of the state on seal(), but rather when the resulting commit
cohort initiates commit -- which means we have to perform three-way
synchronization between seal()/(can|direct)Commit/finishReconnect,
to ensure we flush state towards the backend exactly once.

To do that, we guard the methods involved with locking for split
them into fast/slow paths and add an explicit flushState() method
by which subclasses forward their current unsent state to their
successor. This solution is correct but a bit heavy-handed, so it
will be further optimized in a follow-up patch.

Change-Id: Id5f156dc18faef5b9184c3e2e3d24f7af1b18841
Signed-off-by: Robert Varga <>
6 years agoBUG-5280: TransactionAbortRequest is used for user aborts 30/48830/2
Robert Varga [Wed, 30 Nov 2016 14:24:51 +0000 (15:24 +0100)]
BUG-5280: TransactionAbortRequest is used for user aborts

TransactionAbortRequest is used to indicate both user abort
and 3PC abort, whereas current backend code assumed it is
only used for 3PC -- hence it required a cohort to be present.

Teach handleTransactionAbort to use a direct transaction abort
if it receives a request without having a cohort present.

Change-Id: Ia469d907edb575d5f1ee5e4f630fe1a19204032f
Signed-off-by: Robert Varga <>
6 years agoFix FindBugs warnings in sal-remoterpc-connector and enable enforcement 96/47696/5
Tom Pantelis [Thu, 27 Oct 2016 17:06:33 +0000 (13:06 -0400)]
Fix FindBugs warnings in sal-remoterpc-connector and enable enforcement

Warnings fixed:
  - RemoteRpcImplementation: use of 'error' known to be null
  - RpcBroker, RpcRegistry: The Creator class has non-Serializable field.
    Removed the Creator class and used Props that creates by reflection.
  - RpcBroker: use of 'result' that is marked as @Nullable
  - RpcBroker: redundant check of 'result.getErrors()' that is known to be
    non-null (marked as @Nonnull).
  - Gossiper, RemoteRpcRegistryMXBeanImpl: use entrySet iterator instead of
    keySet and get.
  - Messages: redundant specification of implements Serializable
  - LatestEntryRoutingLogic: Comparator should also implement Serializable in
    case TreeSet is serialized. This isn't the case here but it doesn't hurt
    to implement Serializable in lieu of supressing the warning.
  - LatestEntryRoutingLogic: Fixed potential null pointer de-reference in

Change-Id: I8930c8975e1dd9179d78e74087b3994a365b90f8
Signed-off-by: Tom Pantelis <>
6 years agoFix FindBugs warnings in blueprint and enable enforcement 29/47629/4
Tom Pantelis [Wed, 26 Oct 2016 16:58:05 +0000 (12:58 -0400)]
Fix FindBugs warnings in blueprint and enable enforcement

Warnings fixed:
- OpendaylightNamespaceHandler(line 83): "Usage of GetResource may be unsafe
  if class is extended". Made the class final so it can't be extended.

- BlueprintContainerRestartServiceImpl(line 140): "return value of this method
  should be checked". Log warning if 'await' returns false.

Change-Id: I1473acabd0a4126f5e5d2745292fcbff9a308462
Signed-off-by: Tom Pantelis <>
6 years agoBUG-5280: fix problems identified by integration tests 06/48706/17
Robert Varga [Fri, 25 Nov 2016 15:16:41 +0000 (16:16 +0100)]
BUG-5280: fix problems identified by integration tests

Switching the integration test suite has flushed out couple
of problems in the implementation, notably:

- wrong formatting placeholder
- unhandled requests during replay
- uninitialized path in AbstractReadTransactionRequestProxyV1
- missing sequence number bump in local commit case
- wrong writeObject() in ReadTransactionSuccessProxyV1
- IllegalStateException thrown instead of TransactionChainClosedException
- attempt to create history=0 on the backend
- mismatched sequences during preCommit message replay
- ConcurrentModificationException during localAbort()
- missing upcalls to LocalHistory concretizations when transactions abort
  and complete
- incorrect order on enqueue/send, leading to unpaired responses

Change-Id: I252a795dadb917452b9eb6d591a5c12ca5b69a45
Signed-off-by: Robert Varga <>
6 years agoConstantSchemaAbstractDataBrokerTest, faster than AbstractDataBrokerTest 70/47770/10
Michael Vorburger [Thu, 27 Oct 2016 17:27:36 +0000 (19:27 +0200)]
ConstantSchemaAbstractDataBrokerTest, faster than AbstractDataBrokerTest

In something like AclServiceTest, of which I'm going to be writing a lot
more shortly, which runs e.g. 7 @Test methods under a fresh DataBroker,
there is no need to re-re-re-re do the bloody slow creation of
SchemaContext every time (because its content is based on the classpath,
wihch is fixed for a given single test).

This optimization approx. halfs the time e.g. for AclServiceTest, from
ca. 30s to ca. 15s.

Implemented on the train & ferry traveling back from EclipseCon ;)

Change-Id: Ic47f8abf9833bafcce13655b46cbce3e02aed050
Signed-off-by: Michael Vorburger <>