ISPN-6553 Configuration Serialization #4274

tristantarrant · 2016-04-27T15:24:44Z

https://issues.jboss.org/browse/ISPN-6552
Merge parsers
https://issues.jboss.org/browse/ISPN-6553
Configuration serialization
https://issues.jboss.org/browse/ISPN-6554
XML configuration converter for 6.0 files

slaskawi · 2016-04-29T14:22:16Z

commons/src/main/java/org/infinispan/commons/configuration/attributes/AttributeSet.java

+
+
+   /**
+    * Writes this attributes to the specified XMLStreamWriter


This Javadoc could be probably removed or we should add proper docs to the rest of the methods (I would vote for the latter).

I have added javadocs

slaskawi · 2016-04-29T14:31:12Z

There are some CI failures that seems to be related

slaskawi · 2016-04-29T14:34:05Z

This PR is gigantic. Are there any places where we should focus on?

tristantarrant · 2016-04-29T15:50:25Z

Yes, sorry about the size.

This adds a new "ConfigurationSerializer" interface which is the symmetric implementation of a ConfigurationParser.
AttributeSets now can "write" themselves and some changes have been made to support this. I have changed many attribute names to their xml tag form, aside from the ones that need to be referenced as properties for backwards compatibility (e.g. cachestores)
The Configuration Converter for 6.0 is basically the old parser, which is used to read an old format into an in-memory Configuration and which is then serialized back using the ConfigurationSerializer
I have also merged all versioned parsers into single ones.
I found a bad bug in the ServiceFinder implementation which, when supplied with multiple classloaders, would return multiple instances of services.

pruivo · 2016-05-02T13:07:21Z

commons/src/main/java/org/infinispan/commons/util/ServiceFinder.java

@@ -25,7 +27,7 @@
   private static final Log LOG = LogFactory.getLog(ServiceFinder.class);

   public static <T> Set<T> load(Class<T> contract, ClassLoader... loaders) {


it can return a Collection<T>...

pruivo · 2016-05-06T17:08:56Z

...main/java/org/infinispan/persistence/sifs/configuration/SoftIndexFileStoreConfiguration.java

+   static final AttributeDefinition<Integer> MAX_FILE_SIZE = AttributeDefinition.builder("maxFileSize", 16 * 1024 * 1024).immutable().autoPersist(false).build();
+   static final AttributeDefinition<Integer> MIN_NODE_SIZE = AttributeDefinition.builder("minNodeSize", -1).immutable().autoPersist(false).build();
+   static final AttributeDefinition<Integer> MAX_NODE_SIZE = AttributeDefinition.builder("maxNodeSize", 4096).immutable().autoPersist(false).build();
+   static final AttributeDefinition<Integer> INDEX_QUEUE_LENGTH = AttributeDefinition.builder("indexQueueLength", 1000).immutable().autoPersist(false).build();


missing .xmlName("max-queue-length")

pruivo · 2016-05-06T17:10:47Z

hey @tristantarrant, I've finished my review about the unit test for the cache-stores. it is just a couple of minor fixes :)

tristantarrant · 2016-05-09T10:26:50Z

@pruivo done

tristantarrant · 2016-05-09T10:41:44Z

Right :)

pruivo · 2016-05-09T11:15:41Z

...main/java/org/infinispan/persistence/sifs/configuration/SoftIndexFileStoreConfiguration.java

-   static final AttributeDefinition<Boolean> SYNC_WRITES = AttributeDefinition.builder("syncWrites", false).immutable().build();
+   static final AttributeDefinition<String> DATA_LOCATION = AttributeDefinition.builder("dataLocation", "Infinispan-SoftIndexFileStore-Data").immutable().autoPersist(false).xmlName("path").build();
+   static final AttributeDefinition<String> INDEX_LOCATION = AttributeDefinition.builder("indexLocation", "Infinispan-SoftIndexFileStore-Index").immutable().autoPersist(false).xmlName("path").build();
+   static final AttributeDefinition<Integer> INDEX_SEGMENTS = AttributeDefinition.builder("indexSegments", 3).immutable().autoPersist(false).autoPersist(false).xmlName("segments").build();


nitpick: autoPersist invoked twice.

tristantarrant · 2016-05-09T12:26:16Z

Now ? :)

pruivo · 2016-05-09T13:00:24Z

.../src/main/java/org/infinispan/persistence/jpa/configuration/JpaStoreConfigurationParser.java

-      if (reader.hasNext() && (reader.nextTag() != XMLStreamConstants.END_ELEMENT)) {
-         throw ParseUtils.unexpectedElement(reader);
+      while (reader.hasNext() && (reader.nextTag() != XMLStreamConstants.END_ELEMENT)) {
+         Element element = Element.forName(reader.getLocalName());


not needed.

pruivo · 2016-05-09T13:03:20Z

just on last comment. it looks good to me.
@slaskawi or @wburns do you have any other comment?

slaskawi · 2016-05-10T08:47:48Z

Nope, looks good to me.

pruivo · 2016-05-10T09:41:44Z

integrated! thanks @tristantarrant !

tristantarrant added Ready for Review Urgent! labels Apr 27, 2016

slaskawi reviewed Apr 29, 2016
View reviewed changes

slaskawi added the Changes Suggested label Apr 29, 2016

tristantarrant force-pushed the ISPN-6553/config_serialization branch from 69a3010 to 024c266 Compare April 29, 2016 15:43

tristantarrant removed the Changes Suggested label Apr 29, 2016

tristantarrant force-pushed the ISPN-6553/config_serialization branch from 024c266 to 19fa6c9 Compare May 2, 2016 12:34

pruivo reviewed May 2, 2016
View reviewed changes

pruivo reviewed May 6, 2016
View reviewed changes

tristantarrant force-pushed the ISPN-6553/config_serialization branch 3 times, most recently from 7bae908 to f53885e Compare May 9, 2016 10:25

tristantarrant force-pushed the ISPN-6553/config_serialization branch from f53885e to 5826792 Compare May 9, 2016 10:41

tristantarrant force-pushed the ISPN-6553/config_serialization branch from 5826792 to c03b784 Compare May 9, 2016 10:49

pruivo reviewed May 9, 2016
View reviewed changes

tristantarrant force-pushed the ISPN-6553/config_serialization branch from c03b784 to 201faf5 Compare May 9, 2016 12:25

pruivo reviewed May 9, 2016
View reviewed changes

tristantarrant added 3 commits May 9, 2016 15:15

ISPN-6552 Merge parsers

f5eb4f3

ISPN-6553 Configuration XML Serializer

4720910

ISPN-6554 XML configuration converter for 6.0 files

4dd4f8b

tristantarrant force-pushed the ISPN-6553/config_serialization branch from 201faf5 to 4dd4f8b Compare May 9, 2016 13:16

pruivo closed this May 10, 2016

tristantarrant deleted the ISPN-6553/config_serialization branch September 1, 2016 08:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ISPN-6553 Configuration Serialization #4274

ISPN-6553 Configuration Serialization #4274

tristantarrant commented Apr 27, 2016

slaskawi Apr 29, 2016

tristantarrant Apr 29, 2016

slaskawi commented Apr 29, 2016

slaskawi commented Apr 29, 2016

tristantarrant commented Apr 29, 2016

pruivo May 2, 2016

pruivo May 6, 2016

pruivo commented May 6, 2016

tristantarrant commented May 9, 2016

tristantarrant commented May 9, 2016

pruivo May 9, 2016

tristantarrant commented May 9, 2016

pruivo May 9, 2016

tristantarrant May 9, 2016

pruivo commented May 9, 2016

slaskawi commented May 10, 2016

pruivo commented May 10, 2016



		/**
		* Writes this attributes to the specified XMLStreamWriter

		@@ -25,7 +27,7 @@
		private static final Log LOG = LogFactory.getLog(ServiceFinder.class);

		public static <T> Set<T> load(Class<T> contract, ClassLoader... loaders) {

ISPN-6553 Configuration Serialization #4274

ISPN-6553 Configuration Serialization #4274

Conversation

tristantarrant commented Apr 27, 2016

slaskawi Apr 29, 2016

Choose a reason for hiding this comment

tristantarrant Apr 29, 2016

Choose a reason for hiding this comment

slaskawi commented Apr 29, 2016

slaskawi commented Apr 29, 2016

tristantarrant commented Apr 29, 2016

pruivo May 2, 2016

Choose a reason for hiding this comment

pruivo May 6, 2016

Choose a reason for hiding this comment

pruivo commented May 6, 2016

tristantarrant commented May 9, 2016

tristantarrant commented May 9, 2016

pruivo May 9, 2016

Choose a reason for hiding this comment

tristantarrant commented May 9, 2016

pruivo May 9, 2016

Choose a reason for hiding this comment

tristantarrant May 9, 2016

Choose a reason for hiding this comment

pruivo commented May 9, 2016

slaskawi commented May 10, 2016

pruivo commented May 10, 2016