You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2021/07/08 21:46:19 UTC
[GitHub] [accumulo] EdColeman opened a new pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
EdColeman opened a new pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194
This is the first PR in moving towards a refactored ZooKeeper property
storage. Intented to replace the storing indididual properties in ZooKeeper to be
stored / managed as a group, that is stored on a single node.
- Provides a header that maintains schema version, data version, and timestamp.
- Optional compresson of the byte storage array.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] DomGarguilo commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r667124000
##########
File path: server/base/src/test/java/org/apache/accumulo/server/conf/codec/PropEncodingV1Test.java
##########
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+
+import java.time.Instant;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class PropEncodingV1Test {
+
+ private static final Logger log = LoggerFactory.getLogger(PropEncodingV1Test.class);
+
+ @Test
+ public void defaultConstructor() {
+ PropEncoding props = new PropEncodingV1();
+
+ // on first write, the znode version expected should be 0.
+ // expected should never be -1. That would just overwrite any znode
+ assertEquals(0, props.getExpectedVersion());
+
+ byte[] bytes = props.toBytes();
+ PropEncodingV1 decoded = new PropEncodingV1(bytes);
+
+ // initial version in ZooKeeper expected to be 0.
+ assertEquals(0, decoded.getDataVersion());
+ assertEquals(0, props.getExpectedVersion());
+
+ }
+
+ @Test
+ public void propValidation() {
+ Map<String,String> propMap = new HashMap<>();
+ propMap.put("k1", "v1");
+ propMap.put("k2", "v2");
+
+ PropEncoding props = new PropEncodingV1();
+ props.addProperties(propMap);
+
+ assertEquals(propMap, props.getAllProperties());
+
+ props.addProperty("k3", "v3");
+ assertEquals(3, props.getAllProperties().size());
+
+ props.removeProperty("k2");
+
+ assertEquals(2, props.getAllProperties().size());
+ assertEquals("v1", props.getProperty("k1"));
+ assertNull("v1", props.getProperty("k2"));
+ assertEquals("v3", props.getProperty("k3"));
+
+ }
+
+ @Test
+ public void compressedEncodeTest() {
+
+ PropEncoding props = new PropEncodingV1(1, true, Instant.now());
+ fillMap(props);
+
+ Map<String,String> propValues = props.getAllProperties();
+
+ int ver = props.getDataVersion();
+
+ var createdTs = props.getTimestamp();
Review comment:
Might be clearer this way.
```suggestion
Instant createdTs = props.getTimestamp();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
EdColeman commented on pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#issuecomment-892595808
Closing this and will create a follow-on refactored version that substantially changes the structure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
EdColeman commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r667118871
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncodingV1.java
##########
@@ -0,0 +1,337 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.AbstractMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.TreeMap;
+import java.util.zip.GZIPInputStream;
+import java.util.zip.GZIPOutputStream;
+
+public class PropEncodingV1 implements PropEncoding {
+
+ private Header header;
+
+ private final Map<String,String> props = new HashMap<>();
+
+ /**
+ * Create a default instance, compressed = true, and timestamp = now.
+ */
+ public PropEncodingV1() {
+ this(Integer.MIN_VALUE, true, Instant.now());
+ }
+
+ /**
+ * Instantiate an instance.
+ *
+ * @param dataVersion
+ * should match current zookeeper dataVersion, or a negative value for initial instance.
+ * @param compressed
+ * if true, compress the data.
+ * @param timestamp
+ * timestamp for the data.
+ */
+ public PropEncodingV1(final int dataVersion, final boolean compressed, final Instant timestamp) {
+ header = new Header(dataVersion, timestamp, compressed);
+ }
+
+ /**
+ * (Re) Construct instance from byte array.
+ *
+ * @param bytes
+ * a previously encoded instance in a byte array.
+ */
+ public PropEncodingV1(final byte[] bytes) {
+
+ try (ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
+ DataInputStream dis = new DataInputStream(bis)) {
+
+ header = new Header(dis);
+
+ if (header.isCompressed()) {
+ uncompressProps(bis);
+ } else {
+ readProps(dis);
+ }
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error deserializing properties", ex);
+ }
+ }
+
+ @Override
+ public void addProperty(String k, String v) {
+ props.put(k, v);
+ }
+
+ @Override
+ public void addProperties(Map<String,String> properties) {
+ if (Objects.nonNull(properties)) {
+ props.putAll(properties);
+ }
+ }
+
+ @Override
+ public String getProperty(final String key) {
+ return props.get(key);
+ }
+
+ @Override
+ public Map<String,String> getAllProperties() {
+ return Collections.unmodifiableMap(props);
+ }
+
+ @Override
+ public String removeProperty(final String key) {
+ return props.remove(key);
+ }
+
+ @Override
+ public Instant getTimestamp() {
+ return header.getTimestamp();
+ }
+
+ @Override
+ public int getDataVersion() {
+ return header.getDataVersion();
+ }
+
+ @Override
+ public int getExpectedVersion() {
+ var version = getDataVersion();
Review comment:
Not really - and there may have been a log statement wedged in there that was removed - the assignment is not really needed at all right now - removed it in latest commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] DomGarguilo commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r667122254
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncodingV1.java
##########
@@ -0,0 +1,337 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.AbstractMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.TreeMap;
+import java.util.zip.GZIPInputStream;
+import java.util.zip.GZIPOutputStream;
+
+public class PropEncodingV1 implements PropEncoding {
+
+ private Header header;
+
+ private final Map<String,String> props = new HashMap<>();
+
+ /**
+ * Create a default instance, compressed = true, and timestamp = now.
+ */
+ public PropEncodingV1() {
+ this(Integer.MIN_VALUE, true, Instant.now());
+ }
+
+ /**
+ * Instantiate an instance.
+ *
+ * @param dataVersion
+ * should match current zookeeper dataVersion, or a negative value for initial instance.
+ * @param compressed
+ * if true, compress the data.
+ * @param timestamp
+ * timestamp for the data.
+ */
+ public PropEncodingV1(final int dataVersion, final boolean compressed, final Instant timestamp) {
+ header = new Header(dataVersion, timestamp, compressed);
+ }
+
+ /**
+ * (Re) Construct instance from byte array.
+ *
+ * @param bytes
+ * a previously encoded instance in a byte array.
+ */
+ public PropEncodingV1(final byte[] bytes) {
+
+ try (ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
+ DataInputStream dis = new DataInputStream(bis)) {
+
+ header = new Header(dis);
+
+ if (header.isCompressed()) {
+ uncompressProps(bis);
+ } else {
+ readProps(dis);
+ }
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error deserializing properties", ex);
+ }
+ }
+
+ @Override
+ public void addProperty(String k, String v) {
+ props.put(k, v);
+ }
+
+ @Override
+ public void addProperties(Map<String,String> properties) {
+ if (Objects.nonNull(properties)) {
+ props.putAll(properties);
+ }
+ }
+
+ @Override
+ public String getProperty(final String key) {
+ return props.get(key);
+ }
+
+ @Override
+ public Map<String,String> getAllProperties() {
+ return Collections.unmodifiableMap(props);
+ }
+
+ @Override
+ public String removeProperty(final String key) {
+ return props.remove(key);
+ }
+
+ @Override
+ public Instant getTimestamp() {
+ return header.getTimestamp();
+ }
+
+ @Override
+ public int getDataVersion() {
+ return header.getDataVersion();
+ }
+
+ @Override
+ public int getExpectedVersion() {
+ var version = getDataVersion();
Review comment:
I see you addressed this in af7b741
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] DomGarguilo commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r667101785
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncodingV1.java
##########
@@ -0,0 +1,337 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.AbstractMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.TreeMap;
+import java.util.zip.GZIPInputStream;
+import java.util.zip.GZIPOutputStream;
+
+public class PropEncodingV1 implements PropEncoding {
+
+ private Header header;
+
+ private final Map<String,String> props = new HashMap<>();
+
+ /**
+ * Create a default instance, compressed = true, and timestamp = now.
+ */
+ public PropEncodingV1() {
+ this(Integer.MIN_VALUE, true, Instant.now());
+ }
+
+ /**
+ * Instantiate an instance.
+ *
+ * @param dataVersion
+ * should match current zookeeper dataVersion, or a negative value for initial instance.
+ * @param compressed
+ * if true, compress the data.
+ * @param timestamp
+ * timestamp for the data.
+ */
+ public PropEncodingV1(final int dataVersion, final boolean compressed, final Instant timestamp) {
+ header = new Header(dataVersion, timestamp, compressed);
+ }
+
+ /**
+ * (Re) Construct instance from byte array.
+ *
+ * @param bytes
+ * a previously encoded instance in a byte array.
+ */
+ public PropEncodingV1(final byte[] bytes) {
+
+ try (ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
+ DataInputStream dis = new DataInputStream(bis)) {
+
+ header = new Header(dis);
+
+ if (header.isCompressed()) {
+ uncompressProps(bis);
+ } else {
+ readProps(dis);
+ }
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error deserializing properties", ex);
+ }
+ }
+
+ @Override
+ public void addProperty(String k, String v) {
+ props.put(k, v);
+ }
+
+ @Override
+ public void addProperties(Map<String,String> properties) {
+ if (Objects.nonNull(properties)) {
+ props.putAll(properties);
+ }
+ }
+
+ @Override
+ public String getProperty(final String key) {
+ return props.get(key);
+ }
+
+ @Override
+ public Map<String,String> getAllProperties() {
+ return Collections.unmodifiableMap(props);
+ }
+
+ @Override
+ public String removeProperty(final String key) {
+ return props.remove(key);
+ }
+
+ @Override
+ public Instant getTimestamp() {
+ return header.getTimestamp();
+ }
+
+ @Override
+ public int getDataVersion() {
+ return header.getDataVersion();
+ }
+
+ @Override
+ public int getExpectedVersion() {
+ var version = getDataVersion();
Review comment:
Any reason var is used here instead of int?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] DomGarguilo commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r667098476
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncoding.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.time.Instant;
+import java.util.Map;
+
+public interface PropEncoding {
+
+ /**
+ * Add a property. If the property already exists it is overwritten.
+ *
+ * @param key
+ * the name of the property
+ * @param value
+ * the value of the property.
+ */
+ void addProperty(String key, String value);
+
+ /**
+ * Add multiple properties. If a property already exists it is overwritten.
+ *
+ * @param properties
+ * A map of key, value pairs.
+ */
+ void addProperties(Map<String,String> properties);
+
+ /**
+ * Get a store property or null if it does not exist.
+ *
+ * @param key
+ * the name of the property.
+ * @return the property value.
+ */
+ String getProperty(String key);
+
+ /**
+ * Delete a property.
+ *
+ * @param key
+ * the name of the property.
+ * @return the previous value if the property was present.
+ */
+ String removeProperty(String key);
+
+ /**
+ * Properties are timestamped when the properties are serialized for storage. This is to allow
+ * easy comparison of properties that could have been retrieved at different times.
+ *
+ * @return the timestamp when the properties were serialized.
+ */
+ Instant getTimestamp();
+
+ /**
+ * Properties are store with a data version for serialization. This allows for comparison of
+ * properties and can be used to ensure that vales being written to the backend store have not
+ * changed. The data version is incremented when serialized with toBytes()) so that the instance
Review comment:
```suggestion
* changed. The data version is incremented when serialized with toBytes() so that the instance
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#issuecomment-892902243
Superseded by #2224
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
EdColeman commented on pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#issuecomment-892595808
Closing this and will create a follow-on refactored version that substantially changes the structure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r669948050
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncodingV1.java
##########
@@ -0,0 +1,332 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.AbstractMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.TreeMap;
+import java.util.zip.GZIPInputStream;
+import java.util.zip.GZIPOutputStream;
+
+public class PropEncodingV1 implements PropEncoding {
+
+ private Header header;
+
+ private final Map<String,String> props = new HashMap<>();
+
+ /**
+ * Create a default instance, compressed = true, and timestamp = now.
+ */
+ public PropEncodingV1() {
+ this(Integer.MIN_VALUE, true, Instant.now());
+ }
+
+ /**
+ * Instantiate an instance.
+ *
+ * @param dataVersion
+ * should match current zookeeper dataVersion, or a negative value for initial instance.
+ * @param compressed
+ * if true, compress the data.
+ * @param timestamp
+ * timestamp for the data.
+ */
+ public PropEncodingV1(final int dataVersion, final boolean compressed, final Instant timestamp) {
+ header = new Header(dataVersion, timestamp, compressed);
+ }
+
+ /**
+ * (Re) Construct instance from byte array.
+ *
+ * @param bytes
+ * a previously encoded instance in a byte array.
+ */
+ public PropEncodingV1(final byte[] bytes) {
+
+ try (ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
+ DataInputStream dis = new DataInputStream(bis)) {
+
+ header = new Header(dis);
+
+ if (header.isCompressed()) {
+ uncompressProps(bis);
+ } else {
+ readProps(dis);
+ }
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error deserializing properties", ex);
Review comment:
There exists an UncheckedIOException that would be good to use here instead of IllegalStateException.
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncodingV1.java
##########
@@ -0,0 +1,332 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.AbstractMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.TreeMap;
+import java.util.zip.GZIPInputStream;
+import java.util.zip.GZIPOutputStream;
+
+public class PropEncodingV1 implements PropEncoding {
+
+ private Header header;
+
+ private final Map<String,String> props = new HashMap<>();
+
+ /**
+ * Create a default instance, compressed = true, and timestamp = now.
+ */
+ public PropEncodingV1() {
+ this(Integer.MIN_VALUE, true, Instant.now());
+ }
+
+ /**
+ * Instantiate an instance.
+ *
+ * @param dataVersion
+ * should match current zookeeper dataVersion, or a negative value for initial instance.
+ * @param compressed
+ * if true, compress the data.
+ * @param timestamp
+ * timestamp for the data.
+ */
+ public PropEncodingV1(final int dataVersion, final boolean compressed, final Instant timestamp) {
+ header = new Header(dataVersion, timestamp, compressed);
+ }
+
+ /**
+ * (Re) Construct instance from byte array.
+ *
+ * @param bytes
+ * a previously encoded instance in a byte array.
+ */
+ public PropEncodingV1(final byte[] bytes) {
+
+ try (ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
+ DataInputStream dis = new DataInputStream(bis)) {
+
+ header = new Header(dis);
+
+ if (header.isCompressed()) {
+ uncompressProps(bis);
+ } else {
+ readProps(dis);
+ }
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error deserializing properties", ex);
+ }
+ }
+
+ @Override
+ public void addProperty(String k, String v) {
+ props.put(k, v);
+ }
+
+ @Override
+ public void addProperties(Map<String,String> properties) {
+ if (Objects.nonNull(properties)) {
+ props.putAll(properties);
+ }
+ }
+
+ @Override
+ public String getProperty(final String key) {
+ return props.get(key);
+ }
+
+ @Override
+ public Map<String,String> getAllProperties() {
+ return Collections.unmodifiableMap(props);
+ }
+
+ @Override
+ public String removeProperty(final String key) {
+ return props.remove(key);
+ }
+
+ @Override
+ public Instant getTimestamp() {
+ return header.getTimestamp();
+ }
+
+ @Override
+ public int getDataVersion() {
+ return header.getDataVersion();
+ }
+
+ @Override
+ public int getExpectedVersion() {
+ return Math.max(0, getDataVersion() - 1);
+ }
+
+ @Override
+ public boolean isCompressed() {
+ return header.isCompressed();
+ }
+
+ @Override
+ public byte[] toBytes() {
+
+ try (ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ DataOutputStream dos = new DataOutputStream(bos)) {
+
+ int nextVersion = Math.max(0, header.getDataVersion() + 1);
+
+ header = new Header(nextVersion, Instant.now(), header.isCompressed());
+ header.writeHeader(dos);
+
+ if (header.isCompressed()) {
+ compressProps(bos);
+ } else {
+ writeProps(dos);
+ }
+ dos.flush();
+ return bos.toByteArray();
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error serializing properties", ex);
+ }
+ }
+
+ private void writeProps(final DataOutputStream dos) throws IOException {
+
+ dos.writeInt(props.size());
+
+ props.forEach((k, v) -> writeKV(k, v, dos));
+
+ dos.flush();
+ }
+
+ private void compressProps(final ByteArrayOutputStream bos) {
+
+ try (GZIPOutputStream gzipOut = new GZIPOutputStream(bos);
+ DataOutputStream dos = new DataOutputStream(gzipOut)) {
+
+ writeProps(dos);
+
+ gzipOut.flush();
+ gzipOut.finish();
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error compressing properties", ex);
+ }
+ }
+
+ private void readProps(final DataInputStream dis) throws IOException {
+
+ int items = dis.readInt();
+
+ for (int i = 0; i < items; i++) {
+ Map.Entry<String,String> e = readKV(dis);
+ props.put(e.getKey(), e.getValue());
+ }
+ }
+
+ private void uncompressProps(final ByteArrayInputStream bis) throws IOException {
+
+ try (GZIPInputStream gzipIn = new GZIPInputStream(bis);
+ DataInputStream dis = new DataInputStream(gzipIn)) {
+ readProps(dis);
+ }
+ }
+
+ private void writeKV(final String k, final String v, final DataOutputStream dos) {
+ try {
+ dos.writeUTF(k);
+ dos.writeUTF(v);
+ } catch (IOException ex) {
+ throw new IllegalStateException(
+ String.format("Exception encountered writing props k:'%s', v:'%s", k, v), ex);
+ }
+ }
+
+ private Map.Entry<String,String> readKV(final DataInputStream dis) {
+ try {
+ String k = dis.readUTF();
+ String v = dis.readUTF();
+ return new AbstractMap.SimpleEntry<>(k, v);
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Could not read property key value pair", ex);
+ }
+ }
+
+ @Override
+ public String print(boolean prettyPrint) {
+ StringBuilder sb = new StringBuilder();
+ sb.append("encoding=").append(header.getEncodingVer());
+ pretty(prettyPrint, sb);
+ sb.append("dataVersion=").append(header.getDataVersion());
+ pretty(prettyPrint, sb);
+ sb.append("timestamp=").append(header.getTimestampISO());
+ pretty(prettyPrint, sb);
+
+ Map<String,String> sorted = new TreeMap<>(props);
+ sorted.forEach((k, v) -> {
+ if (prettyPrint) {
+ sb.append(" ");
+ }
+ sb.append(k).append("=").append(v);
+ pretty(prettyPrint, sb);
+ });
+ return sb.toString();
+ }
+
+ private void pretty(final boolean prettyPrint, final StringBuilder sb) {
+ sb.append(prettyPrint ? "\n" : ", ");
+ }
+
+ /**
+ * Serialization metadata. This data should be written / appear in the encoded bytes first so that
+ * decisions can be made that may make deserilization unnecessary.
+ *
+ * The header values are:
+ * <ul>
+ * <li>encodingVersion - allows for future changes to the encoding schema</li>
+ * <li>dataVersion - allows for quick comparison by comparing versions numbers</li>
+ * <li>timestamp - could allow for deconfliction of concurrent updates</li>
+ * <li>compressed - when true, the rest of the payload is compressed</li>
+ * </ul>
+ */
+ private static class Header {
+
+ private final String encodingVer = "1.0";
+ private final int dataVersion;
+ private final Instant timestamp;
+ private final boolean compressed;
+
+ private static final DateTimeFormatter tsFormatter =
+ DateTimeFormatter.ISO_OFFSET_DATE_TIME.withZone(ZoneId.from(ZoneOffset.UTC));
+
+ public Header(final int dataVersion, final Instant timestamp, final boolean compressed) {
+ this.dataVersion = dataVersion;
+ this.timestamp = timestamp;
+ this.compressed = compressed;
+ }
+
+ public Header(final DataInputStream dis) throws IOException {
+
+ // temporary - would need to change if multiple, compatible versions are developed.
+ String ver = dis.readUTF();
+ if (encodingVer.compareTo(ver) != 0) {
+ throw new IllegalStateException(
+ "Invalid encoding version " + ver + ", expected " + encodingVer);
+ }
+ dataVersion = dis.readInt();
+ timestamp = tsFormatter.parse(dis.readUTF(), Instant::from);
+ compressed = dis.readBoolean();
+ }
+
+ public String getEncodingVer() {
+ return encodingVer;
+ }
+
+ /**
+ * Get the data version - -1 signals the data has not been written out.
+ *
+ * @return -1 if initial version, otherwise the current data version.
+ */
+ public int getDataVersion() {
+ if (dataVersion < 0) {
+ return -1;
+ }
+ return dataVersion;
Review comment:
```suggestion
return Math.max(-1, dataVersion); // anything lower than -1 is just -1
```
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncoding.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.time.Instant;
+import java.util.Map;
+
+public interface PropEncoding {
+
+ /**
+ * Add a property. If the property already exists it is overwritten.
+ *
+ * @param key
+ * the name of the property
+ * @param value
+ * the value of the property.
+ */
+ void addProperty(String key, String value);
+
+ /**
+ * Add multiple properties. If a property already exists it is overwritten.
+ *
+ * @param properties
+ * A map of key, value pairs.
+ */
+ void addProperties(Map<String,String> properties);
+
+ /**
+ * Get a store property or null if it does not exist.
+ *
+ * @param key
+ * the name of the property.
+ * @return the property value.
+ */
+ String getProperty(String key);
+
+ /**
+ * Delete a property.
+ *
+ * @param key
+ * the name of the property.
+ * @return the previous value if the property was present.
+ */
+ String removeProperty(String key);
+
Review comment:
I think this interface could be simplified and we limit its role so it's not acting as both a map builder (builder pattern-ish) and a serializer (template method pattern). There are already existing map builder utilities if we need to use them before interfacing with the encoder code (Guava has `ImmutableMapBuilder`, and Java built-in has `Collectors.toMap()`, as well as immutable `Map.of()`)
Instead of having an interface that allows you to add/remove individual map elements, or entire maps, it could focus on doing the serialization/encoding work only.
For example:
```java
PropEncoding encoder = new PropEncodingV1(encodingOptions);
byte[] encodedMapBytes = encoding.encoder(headerValues).encodeMap(myMap);
Map<String,String> decodedMap = encoding.decoder().decodeMap(encodedMapBytes);
assertEquals(myMap, decodedMap);
```
The proposed change above turns your "PropEncoding" type into a Factory for a corresponding pair of "Encoder"/"Decoder" implementing classes which themselves follow a template method pattern. You could get rid of the factory bits and get something slightly different that still follows a template method pattern:
```java
PropEncoding encoder = new PropEncodingV1(encodingOptions);
byte[] encodedMapBytes = encoding.encodeMap(headerValues, myMap);
Map<String,String> decodedMap = encoding.decodeMap(encodedMapBytes);
assertEquals(myMap, decodedMap);
```
In both cases, they drop the map builder bits to simplify the interface, since that can easily be handled by existing utilities and doesn't need to be replicated as part of this interface, which will only bloat its API and broaden its scope of responsibilities. It's probably best to have the interface focus only on the serialization/encoding, given a map that is already built, and deserialization/decoding, given a byte array.
Not storing the props field inside the object as internal state also helps avoid call-order problems due to user error managing the internal state, like calling `removeProperty()` after calling `toBytes()`.
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncodingV1.java
##########
@@ -0,0 +1,332 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.AbstractMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.TreeMap;
+import java.util.zip.GZIPInputStream;
+import java.util.zip.GZIPOutputStream;
+
+public class PropEncodingV1 implements PropEncoding {
+
+ private Header header;
+
+ private final Map<String,String> props = new HashMap<>();
+
+ /**
+ * Create a default instance, compressed = true, and timestamp = now.
+ */
+ public PropEncodingV1() {
+ this(Integer.MIN_VALUE, true, Instant.now());
+ }
+
+ /**
+ * Instantiate an instance.
+ *
+ * @param dataVersion
+ * should match current zookeeper dataVersion, or a negative value for initial instance.
+ * @param compressed
+ * if true, compress the data.
+ * @param timestamp
+ * timestamp for the data.
+ */
+ public PropEncodingV1(final int dataVersion, final boolean compressed, final Instant timestamp) {
+ header = new Header(dataVersion, timestamp, compressed);
+ }
+
+ /**
+ * (Re) Construct instance from byte array.
+ *
+ * @param bytes
+ * a previously encoded instance in a byte array.
+ */
+ public PropEncodingV1(final byte[] bytes) {
+
+ try (ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
+ DataInputStream dis = new DataInputStream(bis)) {
+
+ header = new Header(dis);
+
+ if (header.isCompressed()) {
+ uncompressProps(bis);
+ } else {
+ readProps(dis);
+ }
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error deserializing properties", ex);
+ }
+ }
+
+ @Override
+ public void addProperty(String k, String v) {
+ props.put(k, v);
+ }
+
+ @Override
+ public void addProperties(Map<String,String> properties) {
+ if (Objects.nonNull(properties)) {
+ props.putAll(properties);
+ }
+ }
+
+ @Override
+ public String getProperty(final String key) {
+ return props.get(key);
+ }
+
+ @Override
+ public Map<String,String> getAllProperties() {
+ return Collections.unmodifiableMap(props);
+ }
+
+ @Override
+ public String removeProperty(final String key) {
+ return props.remove(key);
+ }
+
+ @Override
+ public Instant getTimestamp() {
+ return header.getTimestamp();
+ }
+
+ @Override
+ public int getDataVersion() {
+ return header.getDataVersion();
+ }
+
+ @Override
+ public int getExpectedVersion() {
+ return Math.max(0, getDataVersion() - 1);
+ }
+
+ @Override
+ public boolean isCompressed() {
+ return header.isCompressed();
+ }
+
+ @Override
+ public byte[] toBytes() {
+
+ try (ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ DataOutputStream dos = new DataOutputStream(bos)) {
+
+ int nextVersion = Math.max(0, header.getDataVersion() + 1);
+
+ header = new Header(nextVersion, Instant.now(), header.isCompressed());
+ header.writeHeader(dos);
+
+ if (header.isCompressed()) {
+ compressProps(bos);
+ } else {
+ writeProps(dos);
+ }
+ dos.flush();
+ return bos.toByteArray();
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error serializing properties", ex);
+ }
+ }
+
+ private void writeProps(final DataOutputStream dos) throws IOException {
+
+ dos.writeInt(props.size());
+
+ props.forEach((k, v) -> writeKV(k, v, dos));
+
+ dos.flush();
+ }
+
+ private void compressProps(final ByteArrayOutputStream bos) {
+
+ try (GZIPOutputStream gzipOut = new GZIPOutputStream(bos);
+ DataOutputStream dos = new DataOutputStream(gzipOut)) {
+
+ writeProps(dos);
+
+ gzipOut.flush();
+ gzipOut.finish();
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error compressing properties", ex);
+ }
+ }
+
+ private void readProps(final DataInputStream dis) throws IOException {
+
+ int items = dis.readInt();
+
+ for (int i = 0; i < items; i++) {
+ Map.Entry<String,String> e = readKV(dis);
+ props.put(e.getKey(), e.getValue());
+ }
+ }
+
+ private void uncompressProps(final ByteArrayInputStream bis) throws IOException {
+
+ try (GZIPInputStream gzipIn = new GZIPInputStream(bis);
+ DataInputStream dis = new DataInputStream(gzipIn)) {
+ readProps(dis);
+ }
+ }
+
+ private void writeKV(final String k, final String v, final DataOutputStream dos) {
+ try {
+ dos.writeUTF(k);
+ dos.writeUTF(v);
+ } catch (IOException ex) {
+ throw new IllegalStateException(
+ String.format("Exception encountered writing props k:'%s', v:'%s", k, v), ex);
+ }
+ }
+
+ private Map.Entry<String,String> readKV(final DataInputStream dis) {
+ try {
+ String k = dis.readUTF();
+ String v = dis.readUTF();
+ return new AbstractMap.SimpleEntry<>(k, v);
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Could not read property key value pair", ex);
+ }
+ }
+
+ @Override
+ public String print(boolean prettyPrint) {
+ StringBuilder sb = new StringBuilder();
+ sb.append("encoding=").append(header.getEncodingVer());
+ pretty(prettyPrint, sb);
+ sb.append("dataVersion=").append(header.getDataVersion());
+ pretty(prettyPrint, sb);
+ sb.append("timestamp=").append(header.getTimestampISO());
+ pretty(prettyPrint, sb);
+
+ Map<String,String> sorted = new TreeMap<>(props);
+ sorted.forEach((k, v) -> {
+ if (prettyPrint) {
+ sb.append(" ");
+ }
+ sb.append(k).append("=").append(v);
+ pretty(prettyPrint, sb);
+ });
+ return sb.toString();
+ }
+
+ private void pretty(final boolean prettyPrint, final StringBuilder sb) {
+ sb.append(prettyPrint ? "\n" : ", ");
+ }
+
+ /**
+ * Serialization metadata. This data should be written / appear in the encoded bytes first so that
+ * decisions can be made that may make deserilization unnecessary.
+ *
+ * The header values are:
+ * <ul>
+ * <li>encodingVersion - allows for future changes to the encoding schema</li>
+ * <li>dataVersion - allows for quick comparison by comparing versions numbers</li>
+ * <li>timestamp - could allow for deconfliction of concurrent updates</li>
+ * <li>compressed - when true, the rest of the payload is compressed</li>
+ * </ul>
+ */
+ private static class Header {
+
+ private final String encodingVer = "1.0";
+ private final int dataVersion;
+ private final Instant timestamp;
+ private final boolean compressed;
+
+ private static final DateTimeFormatter tsFormatter =
+ DateTimeFormatter.ISO_OFFSET_DATE_TIME.withZone(ZoneId.from(ZoneOffset.UTC));
+
+ public Header(final int dataVersion, final Instant timestamp, final boolean compressed) {
+ this.dataVersion = dataVersion;
+ this.timestamp = timestamp;
+ this.compressed = compressed;
+ }
+
+ public Header(final DataInputStream dis) throws IOException {
+
+ // temporary - would need to change if multiple, compatible versions are developed.
+ String ver = dis.readUTF();
+ if (encodingVer.compareTo(ver) != 0) {
+ throw new IllegalStateException(
+ "Invalid encoding version " + ver + ", expected " + encodingVer);
+ }
+ dataVersion = dis.readInt();
+ timestamp = tsFormatter.parse(dis.readUTF(), Instant::from);
+ compressed = dis.readBoolean();
+ }
+
+ public String getEncodingVer() {
+ return encodingVer;
+ }
+
+ /**
+ * Get the data version - -1 signals the data has not been written out.
+ *
+ * @return -1 if initial version, otherwise the current data version.
+ */
+ public int getDataVersion() {
+ if (dataVersion < 0) {
+ return -1;
+ }
+ return dataVersion;
+ }
+
+ public Instant getTimestamp() {
+ return timestamp;
+ }
+
+ public String getTimestampISO() {
+ return tsFormatter.format(timestamp);
+ }
+
+ public boolean isCompressed() {
+ return compressed;
+ }
+
+ public void writeHeader(final DataOutputStream dos) throws IOException {
Review comment:
I imagine these are public for unit testing. They can probably be package-private.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
EdColeman commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r667111977
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncodingV1.java
##########
@@ -0,0 +1,337 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.AbstractMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.TreeMap;
+import java.util.zip.GZIPInputStream;
+import java.util.zip.GZIPOutputStream;
+
+public class PropEncodingV1 implements PropEncoding {
Review comment:
The intent was to allow for versioning / migration of the serialization in the future - it could have been Impl because currently the is only one, but it seemed to signal that the possibility was there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
EdColeman commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r667114599
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncodingV1.java
##########
@@ -0,0 +1,337 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.AbstractMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.TreeMap;
+import java.util.zip.GZIPInputStream;
+import java.util.zip.GZIPOutputStream;
+
+public class PropEncodingV1 implements PropEncoding {
+
+ private Header header;
+
+ private final Map<String,String> props = new HashMap<>();
+
+ /**
+ * Create a default instance, compressed = true, and timestamp = now.
+ */
+ public PropEncodingV1() {
+ this(Integer.MIN_VALUE, true, Instant.now());
+ }
+
+ /**
+ * Instantiate an instance.
+ *
+ * @param dataVersion
+ * should match current zookeeper dataVersion, or a negative value for initial instance.
+ * @param compressed
+ * if true, compress the data.
+ * @param timestamp
+ * timestamp for the data.
+ */
+ public PropEncodingV1(final int dataVersion, final boolean compressed, final Instant timestamp) {
+ header = new Header(dataVersion, timestamp, compressed);
+ }
+
+ /**
+ * (Re) Construct instance from byte array.
+ *
+ * @param bytes
+ * a previously encoded instance in a byte array.
+ */
+ public PropEncodingV1(final byte[] bytes) {
+
+ try (ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
+ DataInputStream dis = new DataInputStream(bis)) {
+
+ header = new Header(dis);
+
+ if (header.isCompressed()) {
+ uncompressProps(bis);
+ } else {
+ readProps(dis);
+ }
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error deserializing properties", ex);
+ }
+ }
+
+ @Override
+ public void addProperty(String k, String v) {
+ props.put(k, v);
+ }
+
+ @Override
+ public void addProperties(Map<String,String> properties) {
+ if (Objects.nonNull(properties)) {
+ props.putAll(properties);
+ }
+ }
+
+ @Override
+ public String getProperty(final String key) {
+ return props.get(key);
+ }
+
+ @Override
+ public Map<String,String> getAllProperties() {
+ return Collections.unmodifiableMap(props);
+ }
+
+ @Override
+ public String removeProperty(final String key) {
+ return props.remove(key);
+ }
+
+ @Override
+ public Instant getTimestamp() {
+ return header.getTimestamp();
+ }
+
+ @Override
+ public int getDataVersion() {
+ return header.getDataVersion();
+ }
+
+ @Override
+ public int getExpectedVersion() {
+ var version = getDataVersion();
+ return Math.max(0, version - 1);
+ }
+
+ @Override
+ public boolean isCompressed() {
+ return header.isCompressed();
+ }
+
+ @Override
+ public byte[] toBytes() {
+
+ try (ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ DataOutputStream dos = new DataOutputStream(bos)) {
+
+ int nextVersion = Math.max(0, header.getDataVersion() + 1);
+
+ header = new Header(nextVersion, Instant.now(), header.isCompressed());
+ header.writeHeader(dos);
+
+ if (header.isCompressed()) {
+ compressProps(bos);
+ } else {
+ writeProps(dos);
+ }
+ dos.flush();
+ return bos.toByteArray();
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error serializing properties", ex);
+ }
+ }
+
+ private void writeProps(final DataOutputStream dos) throws IOException {
+
+ dos.writeInt(props.size());
+
+ props.forEach((k, v) -> writeKV(k, v, dos));
+
+ dos.flush();
+ }
+
+ private void compressProps(final ByteArrayOutputStream bos) {
+
+ try (GZIPOutputStream gzipOut = new GZIPOutputStream(bos);
+ DataOutputStream dos = new DataOutputStream(gzipOut)) {
+
+ writeProps(dos);
+
+ gzipOut.flush();
+ gzipOut.finish();
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error compressing properties", ex);
+ }
+ }
+
+ private void readProps(final DataInputStream dis) throws IOException {
+
+ int items = dis.readInt();
+
+ for (int i = 0; i < items; i++) {
+ Map.Entry<String,String> e = readKV(dis);
+ props.put(e.getKey(), e.getValue());
+ }
+ }
+
+ private void uncompressProps(final ByteArrayInputStream bis) throws IOException {
+
+ try (GZIPInputStream gzipIn = new GZIPInputStream(bis);
+ DataInputStream dis = new DataInputStream(gzipIn)) {
+ readProps(dis);
+ }
+ }
+
+ private void writeKV(final String k, final String v, final DataOutputStream dos) {
+ try {
+ dos.writeUTF(k);
+ dos.writeUTF(v);
+ } catch (IOException ex) {
+ throw new IllegalStateException(
+ String.format("Exception encountered writing props k:'%s', v:'%s", k, v), ex);
+ }
+ }
+
+ private Map.Entry<String,String> readKV(final DataInputStream dis) {
+ try {
+ String k = dis.readUTF();
+ String v = dis.readUTF();
+ return new AbstractMap.SimpleEntry<>(k, v);
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Could not read property key value pair", ex);
+ }
+ }
+
+ @Override
+ public String print(boolean prettyPrint) {
+ StringBuilder sb = new StringBuilder();
+ sb.append("encoding=").append(header.getEncodingVer());
+ pretty(prettyPrint, sb);
+ sb.append("dataVersion=").append(header.getDataVersion());
+ pretty(prettyPrint, sb);
+ sb.append("timestamp=").append(header.getTimestampISO());
+ pretty(prettyPrint, sb);
+
+ Map<String,String> sorted = new TreeMap<>(props);
+ sorted.forEach((k, v) -> {
+ if (prettyPrint) {
+ sb.append(" ");
+ }
+ sb.append(k).append("=").append(v);
+ pretty(prettyPrint, sb);
+ });
+ return sb.toString();
+ }
+
+ private void pretty(final boolean prettyPrint, final StringBuilder sb) {
+ if (prettyPrint) {
+ sb.append("\n");
+ } else {
+ sb.append(", ");
+ }
+ }
+
+ /**
+ * Serialization metadata. This data should be written / appear in the encoded bytes first so that
+ * decisions can be made that may make deserilization unnecessary.
+ *
+ * The header values are:
+ * <ul>
+ * <li>encodingVersion - allows for future changes to the encoding schema</li>
+ * <li>dataVersion - allows for quick comparison by comparing versions numbers</li>
+ * <li>timestamp - could allow for deconfliction of concurrent updates</li>
+ * <li>compressed - when true, the rest of the payload is compressed</li>
+ * </ul>
+ */
+ private static class Header {
+
+ private final String encodingVer = "1.0";
+ private final int dataVersion;
+ private final Instant timestamp;
+ private final boolean compressed;
+
+ private static final DateTimeFormatter tsFormatter =
+ DateTimeFormatter.ISO_OFFSET_DATE_TIME.withZone(ZoneId.from(ZoneOffset.UTC));
+
+ public Header(final int dataVersion, final Instant timestamp, final boolean compressed) {
+ this.dataVersion = dataVersion;
+ this.timestamp = timestamp;
+ this.compressed = compressed;
+ }
+
+ public Header(final DataInputStream dis) throws IOException {
+
+ // temporary - would need to change if multiple, compatible versions are developed.
+ String ver = dis.readUTF();
+ if (encodingVer.compareTo(ver) != 0) {
+ throw new IllegalStateException(
+ "Invalid encoding version " + ver + ", expected " + encodingVer);
+ }
+ dataVersion = dis.readInt();
+ timestamp = tsFormatter.parse(dis.readUTF(), Instant::from);
+ compressed = dis.readBoolean();
+ }
+
+ public String getEncodingVer() {
+ return encodingVer;
+ }
+
+ /**
+ * Get the data version - -1 signals the data has not been written out.
+ *
+ * @return -1 if initial version, otherwise the current data version.
+ */
+ public int getDataVersion() {
+ if (dataVersion < 0) {
+ return -1;
+ }
+ return dataVersion;
+ }
+
+ public Instant getTimestamp() {
Review comment:
Instant provides for using duration and other timestamp maths that seem to be more readable. And it can have finer granularity than the default millisecond timer (depends on implementation, but at best is not worse)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman closed pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
EdColeman closed pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
EdColeman commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r670046509
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncoding.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.time.Instant;
+import java.util.Map;
+
+public interface PropEncoding {
+
+ /**
+ * Add a property. If the property already exists it is overwritten.
+ *
+ * @param key
+ * the name of the property
+ * @param value
+ * the value of the property.
+ */
+ void addProperty(String key, String value);
+
+ /**
+ * Add multiple properties. If a property already exists it is overwritten.
+ *
+ * @param properties
+ * A map of key, value pairs.
+ */
+ void addProperties(Map<String,String> properties);
+
+ /**
+ * Get a store property or null if it does not exist.
+ *
+ * @param key
+ * the name of the property.
+ * @return the property value.
+ */
+ String getProperty(String key);
+
+ /**
+ * Delete a property.
+ *
+ * @param key
+ * the name of the property.
+ * @return the previous value if the property was present.
+ */
+ String removeProperty(String key);
+
Review comment:
I will look into this - originally I was shooting to have the header state and the properties encapsulated to ease management in the cache. If the cache only maintains a Map with the values then some other operations will need to track the header values separately. The header data version is used to catch overlapping writes to ZooKeeper and should be maintained along with the property collection.
It may be better to separate the serialization (following your suggestion), but also keep the idea of a versioned collection of properties. Would need a better name though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#issuecomment-892902243
Superseded by #2224
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r670109191
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncoding.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.time.Instant;
+import java.util.Map;
+
+public interface PropEncoding {
+
+ /**
+ * Add a property. If the property already exists it is overwritten.
+ *
+ * @param key
+ * the name of the property
+ * @param value
+ * the value of the property.
+ */
+ void addProperty(String key, String value);
+
+ /**
+ * Add multiple properties. If a property already exists it is overwritten.
+ *
+ * @param properties
+ * A map of key, value pairs.
+ */
+ void addProperties(Map<String,String> properties);
+
+ /**
+ * Get a store property or null if it does not exist.
+ *
+ * @param key
+ * the name of the property.
+ * @return the property value.
+ */
+ String getProperty(String key);
+
+ /**
+ * Delete a property.
+ *
+ * @param key
+ * the name of the property.
+ * @return the previous value if the property was present.
+ */
+ String removeProperty(String key);
+
Review comment:
Ah. I think I understand. This is looking a bit "MVC"-like:
1. Model - a versioned properties object (properties map + header info)
2. View - the serialization / encoding stuff (but more than a "view", because it's bidirectional)
3. Controller - the cache that holds a reference to an instance of the data, and triggers the read/write through the encoder
Whether or not the MVC pattern fits exactly, I do think there's some value in trying to keep the "worker" code separate from the data model, which is the core of what I was suggesting above, and I think still holds, even if you're working with a `VersionedProperties` object instead of a `Map<String,String>` object.
I actually like the name `VersionedProperties`, because you can easily have a `getVersion()` method on it that gives you something to compare in the cache. This version would be the "modification version", of course, not the "serialization format version", which could be entirely handled by the serializer/encoder code. The difference is that the "modification version" is relevant to determine whether the properties have changed, and the "serialization format version" has to do with backwards compatibility at the persistence layer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
EdColeman commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r670039779
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncodingV1.java
##########
@@ -0,0 +1,332 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.AbstractMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.TreeMap;
+import java.util.zip.GZIPInputStream;
+import java.util.zip.GZIPOutputStream;
+
+public class PropEncodingV1 implements PropEncoding {
+
+ private Header header;
+
+ private final Map<String,String> props = new HashMap<>();
+
+ /**
+ * Create a default instance, compressed = true, and timestamp = now.
+ */
+ public PropEncodingV1() {
+ this(Integer.MIN_VALUE, true, Instant.now());
+ }
+
+ /**
+ * Instantiate an instance.
+ *
+ * @param dataVersion
+ * should match current zookeeper dataVersion, or a negative value for initial instance.
+ * @param compressed
+ * if true, compress the data.
+ * @param timestamp
+ * timestamp for the data.
+ */
+ public PropEncodingV1(final int dataVersion, final boolean compressed, final Instant timestamp) {
+ header = new Header(dataVersion, timestamp, compressed);
+ }
+
+ /**
+ * (Re) Construct instance from byte array.
+ *
+ * @param bytes
+ * a previously encoded instance in a byte array.
+ */
+ public PropEncodingV1(final byte[] bytes) {
+
+ try (ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
+ DataInputStream dis = new DataInputStream(bis)) {
+
+ header = new Header(dis);
+
+ if (header.isCompressed()) {
+ uncompressProps(bis);
+ } else {
+ readProps(dis);
+ }
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error deserializing properties", ex);
+ }
+ }
+
+ @Override
+ public void addProperty(String k, String v) {
+ props.put(k, v);
+ }
+
+ @Override
+ public void addProperties(Map<String,String> properties) {
+ if (Objects.nonNull(properties)) {
+ props.putAll(properties);
+ }
+ }
+
+ @Override
+ public String getProperty(final String key) {
+ return props.get(key);
+ }
+
+ @Override
+ public Map<String,String> getAllProperties() {
+ return Collections.unmodifiableMap(props);
+ }
+
+ @Override
+ public String removeProperty(final String key) {
+ return props.remove(key);
+ }
+
+ @Override
+ public Instant getTimestamp() {
+ return header.getTimestamp();
+ }
+
+ @Override
+ public int getDataVersion() {
+ return header.getDataVersion();
+ }
+
+ @Override
+ public int getExpectedVersion() {
+ return Math.max(0, getDataVersion() - 1);
+ }
+
+ @Override
+ public boolean isCompressed() {
+ return header.isCompressed();
+ }
+
+ @Override
+ public byte[] toBytes() {
+
+ try (ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ DataOutputStream dos = new DataOutputStream(bos)) {
+
+ int nextVersion = Math.max(0, header.getDataVersion() + 1);
+
+ header = new Header(nextVersion, Instant.now(), header.isCompressed());
+ header.writeHeader(dos);
+
+ if (header.isCompressed()) {
+ compressProps(bos);
+ } else {
+ writeProps(dos);
+ }
+ dos.flush();
+ return bos.toByteArray();
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error serializing properties", ex);
+ }
+ }
+
+ private void writeProps(final DataOutputStream dos) throws IOException {
+
+ dos.writeInt(props.size());
+
+ props.forEach((k, v) -> writeKV(k, v, dos));
+
+ dos.flush();
+ }
+
+ private void compressProps(final ByteArrayOutputStream bos) {
+
+ try (GZIPOutputStream gzipOut = new GZIPOutputStream(bos);
+ DataOutputStream dos = new DataOutputStream(gzipOut)) {
+
+ writeProps(dos);
+
+ gzipOut.flush();
+ gzipOut.finish();
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error compressing properties", ex);
+ }
+ }
+
+ private void readProps(final DataInputStream dis) throws IOException {
+
+ int items = dis.readInt();
+
+ for (int i = 0; i < items; i++) {
+ Map.Entry<String,String> e = readKV(dis);
+ props.put(e.getKey(), e.getValue());
+ }
+ }
+
+ private void uncompressProps(final ByteArrayInputStream bis) throws IOException {
+
+ try (GZIPInputStream gzipIn = new GZIPInputStream(bis);
+ DataInputStream dis = new DataInputStream(gzipIn)) {
+ readProps(dis);
+ }
+ }
+
+ private void writeKV(final String k, final String v, final DataOutputStream dos) {
+ try {
+ dos.writeUTF(k);
+ dos.writeUTF(v);
+ } catch (IOException ex) {
+ throw new IllegalStateException(
+ String.format("Exception encountered writing props k:'%s', v:'%s", k, v), ex);
+ }
+ }
+
+ private Map.Entry<String,String> readKV(final DataInputStream dis) {
+ try {
+ String k = dis.readUTF();
+ String v = dis.readUTF();
+ return new AbstractMap.SimpleEntry<>(k, v);
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Could not read property key value pair", ex);
+ }
+ }
+
+ @Override
+ public String print(boolean prettyPrint) {
+ StringBuilder sb = new StringBuilder();
+ sb.append("encoding=").append(header.getEncodingVer());
+ pretty(prettyPrint, sb);
+ sb.append("dataVersion=").append(header.getDataVersion());
+ pretty(prettyPrint, sb);
+ sb.append("timestamp=").append(header.getTimestampISO());
+ pretty(prettyPrint, sb);
+
+ Map<String,String> sorted = new TreeMap<>(props);
+ sorted.forEach((k, v) -> {
+ if (prettyPrint) {
+ sb.append(" ");
+ }
+ sb.append(k).append("=").append(v);
+ pretty(prettyPrint, sb);
+ });
+ return sb.toString();
+ }
+
+ private void pretty(final boolean prettyPrint, final StringBuilder sb) {
+ sb.append(prettyPrint ? "\n" : ", ");
+ }
+
+ /**
+ * Serialization metadata. This data should be written / appear in the encoded bytes first so that
+ * decisions can be made that may make deserilization unnecessary.
+ *
+ * The header values are:
+ * <ul>
+ * <li>encodingVersion - allows for future changes to the encoding schema</li>
+ * <li>dataVersion - allows for quick comparison by comparing versions numbers</li>
+ * <li>timestamp - could allow for deconfliction of concurrent updates</li>
+ * <li>compressed - when true, the rest of the payload is compressed</li>
+ * </ul>
+ */
+ private static class Header {
+
+ private final String encodingVer = "1.0";
+ private final int dataVersion;
+ private final Instant timestamp;
+ private final boolean compressed;
+
+ private static final DateTimeFormatter tsFormatter =
+ DateTimeFormatter.ISO_OFFSET_DATE_TIME.withZone(ZoneId.from(ZoneOffset.UTC));
+
+ public Header(final int dataVersion, final Instant timestamp, final boolean compressed) {
+ this.dataVersion = dataVersion;
+ this.timestamp = timestamp;
+ this.compressed = compressed;
+ }
+
+ public Header(final DataInputStream dis) throws IOException {
+
+ // temporary - would need to change if multiple, compatible versions are developed.
+ String ver = dis.readUTF();
+ if (encodingVer.compareTo(ver) != 0) {
+ throw new IllegalStateException(
+ "Invalid encoding version " + ver + ", expected " + encodingVer);
+ }
+ dataVersion = dis.readInt();
+ timestamp = tsFormatter.parse(dis.readUTF(), Instant::from);
+ compressed = dis.readBoolean();
+ }
+
+ public String getEncodingVer() {
+ return encodingVer;
+ }
+
+ /**
+ * Get the data version - -1 signals the data has not been written out.
+ *
+ * @return -1 if initial version, otherwise the current data version.
+ */
+ public int getDataVersion() {
+ if (dataVersion < 0) {
+ return -1;
+ }
+ return dataVersion;
Review comment:
I changed this so that it either returns -2 or the number and updated the javadoc . I wanted to avoid using -1 and had changed it else where. If by mistake a -1 gets passed to the ZooKeeper putData as the version it will overwrite the node regardless of the version. Instead, requiring the actual version helps guard against concurrent updates to the ZooKeeper node - ZooKeeper with throw an unexpected version exception / code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] DomGarguilo commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r667096229
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncoding.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.time.Instant;
+import java.util.Map;
+
+public interface PropEncoding {
+
+ /**
+ * Add a property. If the property already exists it is overwritten.
+ *
+ * @param key
+ * the name of the property
+ * @param value
+ * the value of the property.
+ */
+ void addProperty(String key, String value);
+
+ /**
+ * Add multiple properties. If a property already exists it is overwritten.
+ *
+ * @param properties
+ * A map of key, value pairs.
+ */
+ void addProperties(Map<String,String> properties);
+
+ /**
+ * Get a store property or null if it does not exist.
+ *
+ * @param key
+ * the name of the property.
+ * @return the property value.
+ */
+ String getProperty(String key);
+
+ /**
+ * Delete a property.
+ *
+ * @param key
+ * the name of the property.
+ * @return the previous value if the property was present.
+ */
+ String removeProperty(String key);
+
+ /**
+ * Properties are timestamped when the properties are serialized for storage. This is to allow
+ * easy comparison of properties that could have been retrieved at different times.
+ *
+ * @return the timestamp when the properties were serialized.
+ */
+ Instant getTimestamp();
+
+ /**
+ * Properties are store with a data version for serialization. This allows for comparison of
+ * properties and can be used to ensure that vales being written to the backend store have not
+ * changed. The data version is incremented when serialized with toBytes()) so that the instance
+ * value is consistent with the store value.
+ *
+ * @return the data version when the properties were serialized.
+ */
+ int getDataVersion();
+
+ /**
+ * The version is incremented with a call to toBytes() - this method returns the value that should
+ * reflect the current version in the backing store. The expected usage is that the properties
+ * will be serialized inti a byte array with a call {@link #toBytes()} - that will increment the
Review comment:
```suggestion
* will be serialized into a byte array with a call {@link #toBytes()} - that will increment the
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman closed pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
EdColeman closed pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
EdColeman commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r667194477
##########
File path: server/base/src/test/java/org/apache/accumulo/server/conf/codec/PropEncodingV1Test.java
##########
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+
+import java.time.Instant;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class PropEncodingV1Test {
+
+ private static final Logger log = LoggerFactory.getLogger(PropEncodingV1Test.class);
+
+ @Test
+ public void defaultConstructor() {
+ PropEncoding props = new PropEncodingV1();
+
+ // on first write, the znode version expected should be 0.
+ // expected should never be -1. That would just overwrite any znode
+ assertEquals(0, props.getExpectedVersion());
+
+ byte[] bytes = props.toBytes();
+ PropEncodingV1 decoded = new PropEncodingV1(bytes);
+
+ // initial version in ZooKeeper expected to be 0.
+ assertEquals(0, decoded.getDataVersion());
+ assertEquals(0, props.getExpectedVersion());
+
+ }
+
+ @Test
+ public void propValidation() {
+ Map<String,String> propMap = new HashMap<>();
+ propMap.put("k1", "v1");
+ propMap.put("k2", "v2");
+
+ PropEncoding props = new PropEncodingV1();
+ props.addProperties(propMap);
+
+ assertEquals(propMap, props.getAllProperties());
+
+ props.addProperty("k3", "v3");
+ assertEquals(3, props.getAllProperties().size());
+
+ props.removeProperty("k2");
+
+ assertEquals(2, props.getAllProperties().size());
+ assertEquals("v1", props.getProperty("k1"));
+ assertNull("v1", props.getProperty("k2"));
+ assertEquals("v3", props.getProperty("k3"));
+
+ }
+
+ @Test
+ public void compressedEncodeTest() {
+
+ PropEncoding props = new PropEncodingV1(1, true, Instant.now());
+ fillMap(props);
+
+ Map<String,String> propValues = props.getAllProperties();
+
+ int ver = props.getDataVersion();
+
+ var createdTs = props.getTimestamp();
Review comment:
Changed in latest push
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r667096302
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncodingV1.java
##########
@@ -0,0 +1,337 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.AbstractMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.TreeMap;
+import java.util.zip.GZIPInputStream;
+import java.util.zip.GZIPOutputStream;
+
+public class PropEncodingV1 implements PropEncoding {
Review comment:
What do you mean by this name? Does `V1` signify the version that is serialized? Does the name of the class coincide with the `encodingVer` variable below?
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncodingV1.java
##########
@@ -0,0 +1,337 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.AbstractMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.TreeMap;
+import java.util.zip.GZIPInputStream;
+import java.util.zip.GZIPOutputStream;
+
+public class PropEncodingV1 implements PropEncoding {
+
+ private Header header;
+
+ private final Map<String,String> props = new HashMap<>();
+
+ /**
+ * Create a default instance, compressed = true, and timestamp = now.
+ */
+ public PropEncodingV1() {
+ this(Integer.MIN_VALUE, true, Instant.now());
+ }
+
+ /**
+ * Instantiate an instance.
+ *
+ * @param dataVersion
+ * should match current zookeeper dataVersion, or a negative value for initial instance.
+ * @param compressed
+ * if true, compress the data.
+ * @param timestamp
+ * timestamp for the data.
+ */
+ public PropEncodingV1(final int dataVersion, final boolean compressed, final Instant timestamp) {
+ header = new Header(dataVersion, timestamp, compressed);
+ }
+
+ /**
+ * (Re) Construct instance from byte array.
+ *
+ * @param bytes
+ * a previously encoded instance in a byte array.
+ */
+ public PropEncodingV1(final byte[] bytes) {
+
+ try (ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
+ DataInputStream dis = new DataInputStream(bis)) {
+
+ header = new Header(dis);
+
+ if (header.isCompressed()) {
+ uncompressProps(bis);
+ } else {
+ readProps(dis);
+ }
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error deserializing properties", ex);
+ }
+ }
+
+ @Override
+ public void addProperty(String k, String v) {
+ props.put(k, v);
+ }
+
+ @Override
+ public void addProperties(Map<String,String> properties) {
+ if (Objects.nonNull(properties)) {
+ props.putAll(properties);
+ }
+ }
+
+ @Override
+ public String getProperty(final String key) {
+ return props.get(key);
+ }
+
+ @Override
+ public Map<String,String> getAllProperties() {
+ return Collections.unmodifiableMap(props);
+ }
+
+ @Override
+ public String removeProperty(final String key) {
+ return props.remove(key);
+ }
+
+ @Override
+ public Instant getTimestamp() {
+ return header.getTimestamp();
+ }
+
+ @Override
+ public int getDataVersion() {
+ return header.getDataVersion();
+ }
+
+ @Override
+ public int getExpectedVersion() {
+ var version = getDataVersion();
+ return Math.max(0, version - 1);
+ }
+
+ @Override
+ public boolean isCompressed() {
+ return header.isCompressed();
+ }
+
+ @Override
+ public byte[] toBytes() {
+
+ try (ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ DataOutputStream dos = new DataOutputStream(bos)) {
+
+ int nextVersion = Math.max(0, header.getDataVersion() + 1);
+
+ header = new Header(nextVersion, Instant.now(), header.isCompressed());
+ header.writeHeader(dos);
+
+ if (header.isCompressed()) {
+ compressProps(bos);
+ } else {
+ writeProps(dos);
+ }
+ dos.flush();
+ return bos.toByteArray();
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error serializing properties", ex);
+ }
+ }
+
+ private void writeProps(final DataOutputStream dos) throws IOException {
+
+ dos.writeInt(props.size());
+
+ props.forEach((k, v) -> writeKV(k, v, dos));
+
+ dos.flush();
+ }
+
+ private void compressProps(final ByteArrayOutputStream bos) {
+
+ try (GZIPOutputStream gzipOut = new GZIPOutputStream(bos);
+ DataOutputStream dos = new DataOutputStream(gzipOut)) {
+
+ writeProps(dos);
+
+ gzipOut.flush();
+ gzipOut.finish();
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error compressing properties", ex);
+ }
+ }
+
+ private void readProps(final DataInputStream dis) throws IOException {
+
+ int items = dis.readInt();
+
+ for (int i = 0; i < items; i++) {
+ Map.Entry<String,String> e = readKV(dis);
+ props.put(e.getKey(), e.getValue());
+ }
+ }
+
+ private void uncompressProps(final ByteArrayInputStream bis) throws IOException {
+
+ try (GZIPInputStream gzipIn = new GZIPInputStream(bis);
+ DataInputStream dis = new DataInputStream(gzipIn)) {
+ readProps(dis);
+ }
+ }
+
+ private void writeKV(final String k, final String v, final DataOutputStream dos) {
+ try {
+ dos.writeUTF(k);
+ dos.writeUTF(v);
+ } catch (IOException ex) {
+ throw new IllegalStateException(
+ String.format("Exception encountered writing props k:'%s', v:'%s", k, v), ex);
+ }
+ }
+
+ private Map.Entry<String,String> readKV(final DataInputStream dis) {
+ try {
+ String k = dis.readUTF();
+ String v = dis.readUTF();
+ return new AbstractMap.SimpleEntry<>(k, v);
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Could not read property key value pair", ex);
+ }
+ }
+
+ @Override
+ public String print(boolean prettyPrint) {
+ StringBuilder sb = new StringBuilder();
+ sb.append("encoding=").append(header.getEncodingVer());
+ pretty(prettyPrint, sb);
+ sb.append("dataVersion=").append(header.getDataVersion());
+ pretty(prettyPrint, sb);
+ sb.append("timestamp=").append(header.getTimestampISO());
+ pretty(prettyPrint, sb);
+
+ Map<String,String> sorted = new TreeMap<>(props);
+ sorted.forEach((k, v) -> {
+ if (prettyPrint) {
+ sb.append(" ");
+ }
+ sb.append(k).append("=").append(v);
+ pretty(prettyPrint, sb);
+ });
+ return sb.toString();
+ }
+
+ private void pretty(final boolean prettyPrint, final StringBuilder sb) {
+ if (prettyPrint) {
+ sb.append("\n");
+ } else {
+ sb.append(", ");
+ }
+ }
+
+ /**
+ * Serialization metadata. This data should be written / appear in the encoded bytes first so that
+ * decisions can be made that may make deserilization unnecessary.
+ *
+ * The header values are:
+ * <ul>
+ * <li>encodingVersion - allows for future changes to the encoding schema</li>
+ * <li>dataVersion - allows for quick comparison by comparing versions numbers</li>
+ * <li>timestamp - could allow for deconfliction of concurrent updates</li>
+ * <li>compressed - when true, the rest of the payload is compressed</li>
+ * </ul>
+ */
+ private static class Header {
+
+ private final String encodingVer = "1.0";
+ private final int dataVersion;
+ private final Instant timestamp;
+ private final boolean compressed;
+
+ private static final DateTimeFormatter tsFormatter =
+ DateTimeFormatter.ISO_OFFSET_DATE_TIME.withZone(ZoneId.from(ZoneOffset.UTC));
+
+ public Header(final int dataVersion, final Instant timestamp, final boolean compressed) {
+ this.dataVersion = dataVersion;
+ this.timestamp = timestamp;
+ this.compressed = compressed;
+ }
+
+ public Header(final DataInputStream dis) throws IOException {
+
+ // temporary - would need to change if multiple, compatible versions are developed.
+ String ver = dis.readUTF();
+ if (encodingVer.compareTo(ver) != 0) {
+ throw new IllegalStateException(
+ "Invalid encoding version " + ver + ", expected " + encodingVer);
+ }
+ dataVersion = dis.readInt();
+ timestamp = tsFormatter.parse(dis.readUTF(), Instant::from);
+ compressed = dis.readBoolean();
+ }
+
+ public String getEncodingVer() {
+ return encodingVer;
+ }
+
+ /**
+ * Get the data version - -1 signals the data has not been written out.
+ *
+ * @return -1 if initial version, otherwise the current data version.
+ */
+ public int getDataVersion() {
+ if (dataVersion < 0) {
+ return -1;
+ }
+ return dataVersion;
+ }
+
+ public Instant getTimestamp() {
Review comment:
The type `Instant` is interesting, I have never used it. Are you using it for any other functionality beyond the `long` timestamp we typically use in `System.currenttimemillis()`? If we used this instead of `long` it would at least give us stronger types for times.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] foster33 commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
foster33 commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r667115930
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncodingV1.java
##########
@@ -244,11 +244,7 @@ public String print(boolean prettyPrint) {
}
private void pretty(final boolean prettyPrint, final StringBuilder sb) {
- if (prettyPrint) {
- sb.append("\n");
- } else {
- sb.append(", ");
- }
+ prettyPrint ? sb.append("\n") : sb.append(", ");
Review comment:
After talking with @Manno15 this seems to fix the compiler issue.
```suggestion
sb.append(prettyPrint ? "\n" : ", ");
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
EdColeman commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r667111977
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncodingV1.java
##########
@@ -0,0 +1,337 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.AbstractMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.TreeMap;
+import java.util.zip.GZIPInputStream;
+import java.util.zip.GZIPOutputStream;
+
+public class PropEncodingV1 implements PropEncoding {
Review comment:
The intent was to allow for versioning / migration of the serialization in the future - it could have been Impl because currently the is only one, but it seemed to signal that the possibility.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
EdColeman commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r667108109
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncodingV1.java
##########
@@ -0,0 +1,337 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.AbstractMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.TreeMap;
+import java.util.zip.GZIPInputStream;
+import java.util.zip.GZIPOutputStream;
+
+public class PropEncodingV1 implements PropEncoding {
+
+ private Header header;
+
+ private final Map<String,String> props = new HashMap<>();
+
+ /**
+ * Create a default instance, compressed = true, and timestamp = now.
+ */
+ public PropEncodingV1() {
+ this(Integer.MIN_VALUE, true, Instant.now());
+ }
+
+ /**
+ * Instantiate an instance.
+ *
+ * @param dataVersion
+ * should match current zookeeper dataVersion, or a negative value for initial instance.
+ * @param compressed
+ * if true, compress the data.
+ * @param timestamp
+ * timestamp for the data.
+ */
+ public PropEncodingV1(final int dataVersion, final boolean compressed, final Instant timestamp) {
+ header = new Header(dataVersion, timestamp, compressed);
+ }
+
+ /**
+ * (Re) Construct instance from byte array.
+ *
+ * @param bytes
+ * a previously encoded instance in a byte array.
+ */
+ public PropEncodingV1(final byte[] bytes) {
+
+ try (ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
+ DataInputStream dis = new DataInputStream(bis)) {
+
+ header = new Header(dis);
+
+ if (header.isCompressed()) {
+ uncompressProps(bis);
+ } else {
+ readProps(dis);
+ }
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error deserializing properties", ex);
+ }
+ }
+
+ @Override
+ public void addProperty(String k, String v) {
+ props.put(k, v);
+ }
+
+ @Override
+ public void addProperties(Map<String,String> properties) {
+ if (Objects.nonNull(properties)) {
+ props.putAll(properties);
+ }
+ }
+
+ @Override
+ public String getProperty(final String key) {
+ return props.get(key);
+ }
+
+ @Override
+ public Map<String,String> getAllProperties() {
+ return Collections.unmodifiableMap(props);
+ }
+
+ @Override
+ public String removeProperty(final String key) {
+ return props.remove(key);
+ }
+
+ @Override
+ public Instant getTimestamp() {
+ return header.getTimestamp();
+ }
+
+ @Override
+ public int getDataVersion() {
+ return header.getDataVersion();
+ }
+
+ @Override
+ public int getExpectedVersion() {
+ var version = getDataVersion();
+ return Math.max(0, version - 1);
+ }
+
+ @Override
+ public boolean isCompressed() {
+ return header.isCompressed();
+ }
+
+ @Override
+ public byte[] toBytes() {
+
+ try (ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ DataOutputStream dos = new DataOutputStream(bos)) {
+
+ int nextVersion = Math.max(0, header.getDataVersion() + 1);
+
+ header = new Header(nextVersion, Instant.now(), header.isCompressed());
+ header.writeHeader(dos);
+
+ if (header.isCompressed()) {
+ compressProps(bos);
+ } else {
+ writeProps(dos);
+ }
+ dos.flush();
+ return bos.toByteArray();
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error serializing properties", ex);
+ }
+ }
+
+ private void writeProps(final DataOutputStream dos) throws IOException {
+
+ dos.writeInt(props.size());
+
+ props.forEach((k, v) -> writeKV(k, v, dos));
+
+ dos.flush();
+ }
+
+ private void compressProps(final ByteArrayOutputStream bos) {
+
+ try (GZIPOutputStream gzipOut = new GZIPOutputStream(bos);
+ DataOutputStream dos = new DataOutputStream(gzipOut)) {
+
+ writeProps(dos);
+
+ gzipOut.flush();
+ gzipOut.finish();
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error compressing properties", ex);
+ }
+ }
+
+ private void readProps(final DataInputStream dis) throws IOException {
+
+ int items = dis.readInt();
+
+ for (int i = 0; i < items; i++) {
+ Map.Entry<String,String> e = readKV(dis);
+ props.put(e.getKey(), e.getValue());
+ }
+ }
+
+ private void uncompressProps(final ByteArrayInputStream bis) throws IOException {
+
+ try (GZIPInputStream gzipIn = new GZIPInputStream(bis);
+ DataInputStream dis = new DataInputStream(gzipIn)) {
+ readProps(dis);
+ }
+ }
+
+ private void writeKV(final String k, final String v, final DataOutputStream dos) {
+ try {
+ dos.writeUTF(k);
+ dos.writeUTF(v);
+ } catch (IOException ex) {
+ throw new IllegalStateException(
+ String.format("Exception encountered writing props k:'%s', v:'%s", k, v), ex);
+ }
+ }
+
+ private Map.Entry<String,String> readKV(final DataInputStream dis) {
+ try {
+ String k = dis.readUTF();
+ String v = dis.readUTF();
+ return new AbstractMap.SimpleEntry<>(k, v);
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Could not read property key value pair", ex);
+ }
+ }
+
+ @Override
+ public String print(boolean prettyPrint) {
+ StringBuilder sb = new StringBuilder();
+ sb.append("encoding=").append(header.getEncodingVer());
+ pretty(prettyPrint, sb);
+ sb.append("dataVersion=").append(header.getDataVersion());
+ pretty(prettyPrint, sb);
+ sb.append("timestamp=").append(header.getTimestampISO());
+ pretty(prettyPrint, sb);
+
+ Map<String,String> sorted = new TreeMap<>(props);
+ sorted.forEach((k, v) -> {
+ if (prettyPrint) {
+ sb.append(" ");
+ }
+ sb.append(k).append("=").append(v);
+ pretty(prettyPrint, sb);
+ });
+ return sb.toString();
+ }
+
+ private void pretty(final boolean prettyPrint, final StringBuilder sb) {
+ if (prettyPrint) {
+ sb.append("\n");
+ } else {
+ sb.append(", ");
+ }
+ }
Review comment:
I need to revert this - it seems to cause a compiler issue (not a statement)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
dlmarion commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r676528269
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncoding.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.time.Instant;
+import java.util.Map;
+
+public interface PropEncoding {
+
+ /**
+ * Add a property. If the property already exists it is overwritten.
+ *
+ * @param key
+ * the name of the property
+ * @param value
+ * the value of the property.
+ */
+ void addProperty(String key, String value);
+
+ /**
+ * Add multiple properties. If a property already exists it is overwritten.
+ *
+ * @param properties
+ * A map of key, value pairs.
+ */
+ void addProperties(Map<String,String> properties);
+
+ /**
+ * Get a store property or null if it does not exist.
+ *
+ * @param key
+ * the name of the property.
+ * @return the property value.
+ */
+ String getProperty(String key);
+
+ /**
+ * Delete a property.
+ *
+ * @param key
+ * the name of the property.
+ * @return the previous value if the property was present.
+ */
+ String removeProperty(String key);
+
+ /**
+ * Properties are timestamped when the properties are serialized for storage. This is to allow
+ * easy comparison of properties that could have been retrieved at different times.
+ *
+ * @return the timestamp when the properties were serialized.
+ */
+ Instant getTimestamp();
+
+ /**
+ * Properties are store with a data version for serialization. This allows for comparison of
+ * properties and can be used to ensure that vales being written to the backend store have not
+ * changed. The data version is incremented when serialized with toBytes() so that the instance
+ * value is consistent with the store value.
+ *
+ * @return the data version when the properties were serialized.
+ */
+ int getDataVersion();
+
+ /**
+ * The version is incremented with a call to toBytes() - this method returns the value that should
+ * reflect the current version in the backing store. The expected usage is that the properties
+ * will be serialized into a byte array with a call {@link #toBytes()} - that will increment the
+ * data version - then, the {@link #getExpectedVersion()} will reflect the value that is currently
+ * in zookeeper and is the value passed to the zookeeper setData methods.
+ *
+ * @return the expected version current in the backend store.
+ */
+ int getExpectedVersion();
+
+ /**
+ * Serialize the version information and the properties.
+ *
+ * @return an array of bytes for storage.
+ */
+ byte[] toBytes();
+
+ /**
+ * Provide user-friend display string.
+ *
+ * @param prettyPrint
+ * if true, insert new lines to improve readability.
+ * @return a formatted string, with optional new lines.
+ */
+ String print(boolean prettyPrint);
+
+ /**
+ * Get an unmodifiable map with all of the property, values.
+ *
+ * @return An unmodifiable view of the property key, values.
+ */
+ Map<String,String> getAllProperties();
+
+ /**
+ * Determine if the stage of the props is compressed or not.
Review comment:
Did you mean `state` instead of `stage`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] foster33 commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
foster33 commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r667092164
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncodingV1.java
##########
@@ -0,0 +1,337 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.AbstractMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.TreeMap;
+import java.util.zip.GZIPInputStream;
+import java.util.zip.GZIPOutputStream;
+
+public class PropEncodingV1 implements PropEncoding {
+
+ private Header header;
+
+ private final Map<String,String> props = new HashMap<>();
+
+ /**
+ * Create a default instance, compressed = true, and timestamp = now.
+ */
+ public PropEncodingV1() {
+ this(Integer.MIN_VALUE, true, Instant.now());
+ }
+
+ /**
+ * Instantiate an instance.
+ *
+ * @param dataVersion
+ * should match current zookeeper dataVersion, or a negative value for initial instance.
+ * @param compressed
+ * if true, compress the data.
+ * @param timestamp
+ * timestamp for the data.
+ */
+ public PropEncodingV1(final int dataVersion, final boolean compressed, final Instant timestamp) {
+ header = new Header(dataVersion, timestamp, compressed);
+ }
+
+ /**
+ * (Re) Construct instance from byte array.
+ *
+ * @param bytes
+ * a previously encoded instance in a byte array.
+ */
+ public PropEncodingV1(final byte[] bytes) {
+
+ try (ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
+ DataInputStream dis = new DataInputStream(bis)) {
+
+ header = new Header(dis);
+
+ if (header.isCompressed()) {
+ uncompressProps(bis);
+ } else {
+ readProps(dis);
+ }
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error deserializing properties", ex);
+ }
+ }
+
+ @Override
+ public void addProperty(String k, String v) {
+ props.put(k, v);
+ }
+
+ @Override
+ public void addProperties(Map<String,String> properties) {
+ if (Objects.nonNull(properties)) {
+ props.putAll(properties);
+ }
+ }
+
+ @Override
+ public String getProperty(final String key) {
+ return props.get(key);
+ }
+
+ @Override
+ public Map<String,String> getAllProperties() {
+ return Collections.unmodifiableMap(props);
+ }
+
+ @Override
+ public String removeProperty(final String key) {
+ return props.remove(key);
+ }
+
+ @Override
+ public Instant getTimestamp() {
+ return header.getTimestamp();
+ }
+
+ @Override
+ public int getDataVersion() {
+ return header.getDataVersion();
+ }
+
+ @Override
+ public int getExpectedVersion() {
+ var version = getDataVersion();
+ return Math.max(0, version - 1);
+ }
+
+ @Override
+ public boolean isCompressed() {
+ return header.isCompressed();
+ }
+
+ @Override
+ public byte[] toBytes() {
+
+ try (ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ DataOutputStream dos = new DataOutputStream(bos)) {
+
+ int nextVersion = Math.max(0, header.getDataVersion() + 1);
+
+ header = new Header(nextVersion, Instant.now(), header.isCompressed());
+ header.writeHeader(dos);
+
+ if (header.isCompressed()) {
+ compressProps(bos);
+ } else {
+ writeProps(dos);
+ }
+ dos.flush();
+ return bos.toByteArray();
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error serializing properties", ex);
+ }
+ }
+
+ private void writeProps(final DataOutputStream dos) throws IOException {
+
+ dos.writeInt(props.size());
+
+ props.forEach((k, v) -> writeKV(k, v, dos));
+
+ dos.flush();
+ }
+
+ private void compressProps(final ByteArrayOutputStream bos) {
+
+ try (GZIPOutputStream gzipOut = new GZIPOutputStream(bos);
+ DataOutputStream dos = new DataOutputStream(gzipOut)) {
+
+ writeProps(dos);
+
+ gzipOut.flush();
+ gzipOut.finish();
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Encountered error compressing properties", ex);
+ }
+ }
+
+ private void readProps(final DataInputStream dis) throws IOException {
+
+ int items = dis.readInt();
+
+ for (int i = 0; i < items; i++) {
+ Map.Entry<String,String> e = readKV(dis);
+ props.put(e.getKey(), e.getValue());
+ }
+ }
+
+ private void uncompressProps(final ByteArrayInputStream bis) throws IOException {
+
+ try (GZIPInputStream gzipIn = new GZIPInputStream(bis);
+ DataInputStream dis = new DataInputStream(gzipIn)) {
+ readProps(dis);
+ }
+ }
+
+ private void writeKV(final String k, final String v, final DataOutputStream dos) {
+ try {
+ dos.writeUTF(k);
+ dos.writeUTF(v);
+ } catch (IOException ex) {
+ throw new IllegalStateException(
+ String.format("Exception encountered writing props k:'%s', v:'%s", k, v), ex);
+ }
+ }
+
+ private Map.Entry<String,String> readKV(final DataInputStream dis) {
+ try {
+ String k = dis.readUTF();
+ String v = dis.readUTF();
+ return new AbstractMap.SimpleEntry<>(k, v);
+
+ } catch (IOException ex) {
+ throw new IllegalStateException("Could not read property key value pair", ex);
+ }
+ }
+
+ @Override
+ public String print(boolean prettyPrint) {
+ StringBuilder sb = new StringBuilder();
+ sb.append("encoding=").append(header.getEncodingVer());
+ pretty(prettyPrint, sb);
+ sb.append("dataVersion=").append(header.getDataVersion());
+ pretty(prettyPrint, sb);
+ sb.append("timestamp=").append(header.getTimestampISO());
+ pretty(prettyPrint, sb);
+
+ Map<String,String> sorted = new TreeMap<>(props);
+ sorted.forEach((k, v) -> {
+ if (prettyPrint) {
+ sb.append(" ");
+ }
+ sb.append(k).append("=").append(v);
+ pretty(prettyPrint, sb);
+ });
+ return sb.toString();
+ }
+
+ private void pretty(final boolean prettyPrint, final StringBuilder sb) {
+ if (prettyPrint) {
+ sb.append("\n");
+ } else {
+ sb.append(", ");
+ }
+ }
Review comment:
I noticed that `pretty` could be condensed a little bit. Obviously not a big deal, but just wanted to give some potential feedback.
```suggestion
private void pretty(final boolean prettyPrint, final StringBuilder sb) {
prettyPrint ? sb.append("\n") : sb.append(", ");
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
dlmarion commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r676529068
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncoding.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.time.Instant;
+import java.util.Map;
+
+public interface PropEncoding {
+
+ /**
+ * Add a property. If the property already exists it is overwritten.
+ *
+ * @param key
+ * the name of the property
+ * @param value
+ * the value of the property.
+ */
+ void addProperty(String key, String value);
+
+ /**
+ * Add multiple properties. If a property already exists it is overwritten.
+ *
+ * @param properties
+ * A map of key, value pairs.
+ */
+ void addProperties(Map<String,String> properties);
+
+ /**
+ * Get a store property or null if it does not exist.
+ *
+ * @param key
+ * the name of the property.
+ * @return the property value.
+ */
+ String getProperty(String key);
+
+ /**
+ * Delete a property.
+ *
+ * @param key
+ * the name of the property.
+ * @return the previous value if the property was present.
+ */
+ String removeProperty(String key);
+
+ /**
+ * Properties are timestamped when the properties are serialized for storage. This is to allow
+ * easy comparison of properties that could have been retrieved at different times.
+ *
+ * @return the timestamp when the properties were serialized.
+ */
+ Instant getTimestamp();
+
+ /**
+ * Properties are store with a data version for serialization. This allows for comparison of
+ * properties and can be used to ensure that vales being written to the backend store have not
+ * changed. The data version is incremented when serialized with toBytes() so that the instance
+ * value is consistent with the store value.
+ *
+ * @return the data version when the properties were serialized.
+ */
+ int getDataVersion();
+
+ /**
+ * The version is incremented with a call to toBytes() - this method returns the value that should
+ * reflect the current version in the backing store. The expected usage is that the properties
+ * will be serialized into a byte array with a call {@link #toBytes()} - that will increment the
+ * data version - then, the {@link #getExpectedVersion()} will reflect the value that is currently
+ * in zookeeper and is the value passed to the zookeeper setData methods.
+ *
+ * @return the expected version current in the backend store.
+ */
+ int getExpectedVersion();
+
+ /**
+ * Serialize the version information and the properties.
+ *
+ * @return an array of bytes for storage.
+ */
+ byte[] toBytes();
+
+ /**
+ * Provide user-friend display string.
+ *
+ * @param prettyPrint
+ * if true, insert new lines to improve readability.
+ * @return a formatted string, with optional new lines.
+ */
+ String print(boolean prettyPrint);
+
+ /**
+ * Get an unmodifiable map with all of the property, values.
+ *
+ * @return An unmodifiable view of the property key, values.
+ */
+ Map<String,String> getAllProperties();
+
+ /**
+ * Determine if the stage of the props is compressed or not.
+ *
+ * @return true if the underlying encoded props are compressed when stored.
+ */
+ boolean isCompressed();
Review comment:
If the properties are written in a compressed format, does that preclude someone from modifying the properties in ZooKeeper directly in an emergency situation?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] DomGarguilo commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r667096482
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncoding.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.time.Instant;
+import java.util.Map;
+
+public interface PropEncoding {
+
+ /**
+ * Add a property. If the property already exists it is overwritten.
+ *
+ * @param key
+ * the name of the property
+ * @param value
+ * the value of the property.
+ */
+ void addProperty(String key, String value);
+
+ /**
+ * Add multiple properties. If a property already exists it is overwritten.
+ *
+ * @param properties
+ * A map of key, value pairs.
+ */
+ void addProperties(Map<String,String> properties);
+
+ /**
+ * Get a store property or null if it does not exist.
+ *
+ * @param key
+ * the name of the property.
+ * @return the property value.
+ */
+ String getProperty(String key);
+
+ /**
+ * Delete a property.
+ *
+ * @param key
+ * the name of the property.
+ * @return the previous value if the property was present.
+ */
+ String removeProperty(String key);
+
+ /**
+ * Properties are timestamped when the properties are serialized for storage. This is to allow
+ * easy comparison of properties that could have been retrieved at different times.
+ *
+ * @return the timestamp when the properties were serialized.
+ */
+ Instant getTimestamp();
+
+ /**
+ * Properties are store with a data version for serialization. This allows for comparison of
+ * properties and can be used to ensure that vales being written to the backend store have not
+ * changed. The data version is incremented when serialized with toBytes()) so that the instance
+ * value is consistent with the store value.
+ *
+ * @return the data version when the properties were serialized.
+ */
+ int getDataVersion();
+
+ /**
+ * The version is incremented with a call to toBytes() - this method returns the value that should
+ * reflect the current version in the backing store. The expected usage is that the properties
+ * will be serialized inti a byte array with a call {@link #toBytes()} - that will increment the
+ * data version - then, the {@link #getExpectedVersion()} will reflect the value that is currently
+ * in zookeeper and is the value passed to the zookeeper setData methods.
+ *
+ * @return the expected version current in the backend store.
+ */
+ int getExpectedVersion();
+
+ /**
+ * Serialize the version information an the properties.
Review comment:
```suggestion
* Serialize the version information and the properties.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on a change in pull request #2194: Serialization of a map of key, value pairs map to / from byte array.
Posted by GitBox <gi...@apache.org>.
EdColeman commented on a change in pull request #2194:
URL: https://github.com/apache/accumulo/pull/2194#discussion_r676592787
##########
File path: server/base/src/main/java/org/apache/accumulo/server/conf/codec/PropEncoding.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.conf.codec;
+
+import java.time.Instant;
+import java.util.Map;
+
+public interface PropEncoding {
+
+ /**
+ * Add a property. If the property already exists it is overwritten.
+ *
+ * @param key
+ * the name of the property
+ * @param value
+ * the value of the property.
+ */
+ void addProperty(String key, String value);
+
+ /**
+ * Add multiple properties. If a property already exists it is overwritten.
+ *
+ * @param properties
+ * A map of key, value pairs.
+ */
+ void addProperties(Map<String,String> properties);
+
+ /**
+ * Get a store property or null if it does not exist.
+ *
+ * @param key
+ * the name of the property.
+ * @return the property value.
+ */
+ String getProperty(String key);
+
+ /**
+ * Delete a property.
+ *
+ * @param key
+ * the name of the property.
+ * @return the previous value if the property was present.
+ */
+ String removeProperty(String key);
+
+ /**
+ * Properties are timestamped when the properties are serialized for storage. This is to allow
+ * easy comparison of properties that could have been retrieved at different times.
+ *
+ * @return the timestamp when the properties were serialized.
+ */
+ Instant getTimestamp();
+
+ /**
+ * Properties are store with a data version for serialization. This allows for comparison of
+ * properties and can be used to ensure that vales being written to the backend store have not
+ * changed. The data version is incremented when serialized with toBytes() so that the instance
+ * value is consistent with the store value.
+ *
+ * @return the data version when the properties were serialized.
+ */
+ int getDataVersion();
+
+ /**
+ * The version is incremented with a call to toBytes() - this method returns the value that should
+ * reflect the current version in the backing store. The expected usage is that the properties
+ * will be serialized into a byte array with a call {@link #toBytes()} - that will increment the
+ * data version - then, the {@link #getExpectedVersion()} will reflect the value that is currently
+ * in zookeeper and is the value passed to the zookeeper setData methods.
+ *
+ * @return the expected version current in the backend store.
+ */
+ int getExpectedVersion();
+
+ /**
+ * Serialize the version information and the properties.
+ *
+ * @return an array of bytes for storage.
+ */
+ byte[] toBytes();
+
+ /**
+ * Provide user-friend display string.
+ *
+ * @param prettyPrint
+ * if true, insert new lines to improve readability.
+ * @return a formatted string, with optional new lines.
+ */
+ String print(boolean prettyPrint);
+
+ /**
+ * Get an unmodifiable map with all of the property, values.
+ *
+ * @return An unmodifiable view of the property key, values.
+ */
+ Map<String,String> getAllProperties();
+
+ /**
+ * Determine if the stage of the props is compressed or not.
+ *
+ * @return true if the underlying encoded props are compressed when stored.
+ */
+ boolean isCompressed();
Review comment:
Yes - There will be a utility to print the values. I'll look at adding the ability to use it to set values if the shell is not sufficient.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org