fix versionstamp offset and bindingtester

Author: Vincent Rouillé

foundationdb-bindingtester/src/main.rs

@@ -23,11 +23,14 @@ static RESULT_NOT_PRESENT: Element = Element::Bytes(Bytes(Cow::Borrowed(b"RESULT
 static GOT_READ_VERSION: Element = Element::Bytes(Bytes(Cow::Borrowed(b"GOT_READ_VERSION")));
 static GOT_COMMITTED_VERSION: Element =
     Element::Bytes(Bytes(Cow::Borrowed(b"GOT_COMMITTED_VERSION")));
+static GOT_APPROXIMATE_SIZE: Element =
+    Element::Bytes(Bytes(Cow::Borrowed(b"GOT_APPROXIMATE_SIZE")));
 static ERROR_NONE: Element = Element::Bytes(Bytes(Cow::Borrowed(b"ERROR: NONE")));
 static ERROR_MULTIPLE: Element = Element::Bytes(Bytes(Cow::Borrowed(b"ERROR: MULTIPLE")));
 static OK: Element = Element::Bytes(Bytes(Cow::Borrowed(b"OK")));
 
 use crate::fdb::options::{MutationType, StreamingMode};
+use tuple::VersionstampOffset;
 fn mutation_from_str(s: &str) -> MutationType {
     match s {
         "ADD" => MutationType::Add,
@@ -163,6 +166,7 @@ enum InstrCode {
     Reset,
     Cancel,
     GetCommittedVersion,
+    GetApproximateSize,
     WaitFuture,
 
     TuplePack,
@@ -238,6 +242,7 @@ impl Instr {
             "RESET" => Reset,
             "CANCEL" => Cancel,
             "GET_COMMITTED_VERSION" => GetCommittedVersion,
+            "GET_APPROXIMATE_SIZE" => GetApproximateSize,
             "WAIT_FUTURE" => WaitFuture,
 
             "TUPLE_PACK" => TuplePack,
@@ -399,7 +404,6 @@ impl TransactionState {
     fn as_mut(&mut self) -> &mut Transaction {
         use TransactionState as S;
 
-        println!("as_mut {:?}", self);
         self.reset();
         match *self {
             S::Transaction(ref mut tr) => tr,
@@ -414,7 +418,6 @@ impl TransactionState {
     fn take(&mut self, id: usize) -> Transaction {
         use TransactionState as S;
 
-        println!("take {:?}", self);
         self.reset();
         match std::mem::replace(self, S::Dead) {
             S::Transaction(tr) => {
@@ -1190,6 +1193,18 @@ impl StackMachine {
                 }
             }
 
+            // Calls get_approximate_size and pushes the byte string "GOT_APPROXIMATE_SIZE"
+            // onto the stack. Note bindings may issue GET_RANGE calls with different
+            // limits, so these bindings can obtain different sizes back.
+            GetApproximateSize => {
+                debug!("get_approximate_size");
+                trx.as_mut()
+                    .get_approximate_size()
+                    .await
+                    .expect("failed to get approximate size");
+                self.push(number, GOT_APPROXIMATE_SIZE.clone().into_owned());
+            }
+
             // Pops the top item off the stack and pushes it back on. If the top item on
             // the stack is a future, this will have the side effect of waiting on the
             // result of the future and pushing the result on the stack. Does not change
@@ -1212,7 +1227,6 @@ impl StackMachine {
                     buf.push(element);
                 }
                 let tuple = Element::Tuple(buf);
-                debug!("tuple_packed {}", Bytes::from(pack(&tuple)));
                 self.push(number, Element::Bytes(pack(&tuple).into()));
             }
 
@@ -1238,21 +1252,25 @@ impl StackMachine {
                     buf.push(element);
                 }
 
-                let tuple = Element::Tuple(buf);
+                let tuple = Element::Tuple(buf.clone());
                 let i = tuple.count_incomplete_versionstamp();
-
-                self.push(
-                    number,
-                    if i == 0 {
-                        ERROR_NONE.clone().into_owned()
-                    } else if i > 0 {
-                        ERROR_MULTIPLE.clone().into_owned()
-                    } else {
-                        OK.clone().into_owned()
-                    },
-                );
-                if i == 1 {
-                    self.push(number, Element::Bytes(pack(&tuple).into()));
+                let mut vec = prefix.into_owned();
+                let offset = buf.pack_into_vec_with_versionstamp(&mut vec);
+                match offset {
+                    VersionstampOffset::None { size: _ } => {
+                        assert_eq!(i, 0);
+                        self.push(number, ERROR_NONE.clone().into_owned());
+                    }
+                    VersionstampOffset::OneIncomplete { offset: _ } => {
+                        assert_eq!(i, 1);
+                        let data = Element::Bytes(vec.into());
+                        self.push(number, OK.clone().into_owned());
+                        self.push(number, data);
+                    }
+                    VersionstampOffset::MultipleIncomplete => {
+                        assert!(i > 1);
+                        self.push(number, ERROR_MULTIPLE.clone().into_owned());
+                    }
                 }
             }
 

foundationdb/src/tuple/mod.rs

@@ -208,7 +208,10 @@ pub fn pack_into<T: TuplePack>(v: &T, output: &mut Vec<u8>) {
 ///
 /// Panics if there is multiple versionstamp present or if the encoded data size doesn't fit in `u32`.
 pub fn pack_into_with_versionstamp<T: TuplePack>(v: &T, output: &mut Vec<u8>) {
-    v.pack_into_vec_with_versionstamp(output)
+    let offset = v.pack_into_vec_with_versionstamp(output);
+    if let VersionstampOffset::MultipleIncomplete = offset {
+        panic!("pack_into_with_versionstamp does not allow multiple versionstamps");
+    }
 }
 
 /// Unpack input

foundationdb/src/tuple/pack.rs

@@ -7,8 +7,8 @@ use std::mem;
 #[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
 pub enum VersionstampOffset {
     None { size: u32 },
-    One { offset: u32 },
-    Multiple,
+    OneIncomplete { offset: u32 },
+    MultipleIncomplete,
 }
 impl std::ops::AddAssign<u32> for VersionstampOffset {
     fn add_assign(&mut self, r: u32) {
@@ -23,14 +23,18 @@ impl std::ops::AddAssign for VersionstampOffset {
             (VersionstampOffset::None { size }, VersionstampOffset::None { size: r }) => {
                 *size += r;
             }
-            (VersionstampOffset::None { size }, VersionstampOffset::One { offset }) => {
-                *self = VersionstampOffset::One {
+            (VersionstampOffset::None { size }, VersionstampOffset::OneIncomplete { offset }) => {
+                *self = VersionstampOffset::OneIncomplete {
                     offset: *size + offset,
                 };
             }
-            (VersionstampOffset::One { .. }, VersionstampOffset::One { .. })
-            | (VersionstampOffset::One { .. }, VersionstampOffset::Multiple) => {
-                *self = VersionstampOffset::Multiple;
+            (
+                VersionstampOffset::OneIncomplete { .. },
+                VersionstampOffset::OneIncomplete { .. },
+            )
+            | (VersionstampOffset::None { .. }, VersionstampOffset::MultipleIncomplete)
+            | (VersionstampOffset::OneIncomplete { .. }, VersionstampOffset::MultipleIncomplete) => {
+                *self = VersionstampOffset::MultipleIncomplete;
             }
             _ => {}
         }
@@ -69,7 +73,10 @@ pub trait TuplePack {
     /// Panics if there is multiple versionstamp present or if the encoded data size doesn't fit in `u32`.
     fn pack_to_vec_with_versionstamp(&self) -> Vec<u8> {
         let mut vec = Vec::new();
-        self.pack_into_vec_with_versionstamp(&mut vec);
+        let offset = self.pack_into_vec_with_versionstamp(&mut vec);
+        if let VersionstampOffset::MultipleIncomplete = offset {
+            panic!("pack_to_vec_with_versionstamp does not allow multiple versionstamps");
+        }
         vec
     }
 
@@ -86,21 +93,16 @@ pub trait TuplePack {
     ///
     /// # Panics
     ///
-    /// Panics if there is multiple versionstamp present or if the encoded data size doesn't fit in `u32`.
-    fn pack_into_vec_with_versionstamp(&self, output: &mut Vec<u8>) {
+    /// Panics if the encoded data size doesn't fit in `u32`.
+    fn pack_into_vec_with_versionstamp(&self, output: &mut Vec<u8>) -> VersionstampOffset {
         let mut offset = VersionstampOffset::None {
             size: u32::try_from(output.len()).expect(PACK_ERR_MSG),
         };
         offset += self.pack_root(output).expect(PACK_ERR_MSG);
-        match offset {
-            VersionstampOffset::None { size: _ } => {}
-            VersionstampOffset::One { offset } => {
-                output.extend_from_slice(&offset.to_le_bytes());
-            }
-            VersionstampOffset::Multiple => {
-                panic!("pack_into_with_versionstamp does not allow multiple versionstamps");
-            }
+        if let VersionstampOffset::OneIncomplete { offset } = offset {
+            output.extend_from_slice(&offset.to_le_bytes());
         }
+        offset
     }
 }
 
@@ -342,14 +344,16 @@ macro_rules! impl_ux {
                 const SZ: usize = mem::size_of::<$ux>();
                 let u = *self;
                 let n = SZ - (u.leading_zeros() as usize) / 8;
+                let mut offset = VersionstampOffset::None { size: n as u32 + 1 };
                 if SZ <= MAX_SZ || n <= MAX_SZ {
                     w.write_all(&[INTZERO + n as u8])?;
                 } else {
                     w.write_all(&[POSINTEND, n as u8])?;
-                }
+                    offset += 1;
+                };
                 w.write_all(&u.to_be_bytes()[SZ - n..])?;
 
-                Ok(VersionstampOffset::None { size: n as u32 + 1 })
+                Ok(offset)
             }
         }
 
@@ -393,24 +397,27 @@ macro_rules! impl_ix {
                 let i = *self;
                 let u = self.wrapping_abs() as $ux;
                 let n = SZ - (u.leading_zeros() as usize) / 8;
+                let mut offset = VersionstampOffset::None { size: n as u32 + 1 };
                 let arr = if i >= 0 {
                     if SZ <= MAX_SZ || n <= MAX_SZ {
                         w.write_all(&[INTZERO + n as u8])?;
                     } else {
                         w.write_all(&[POSINTEND, n as u8])?;
+                        offset += 1;
                     }
                     (u.to_be_bytes())
                 } else {
                     if SZ <= MAX_SZ || n <= MAX_SZ {
                         w.write_all(&[INTZERO - n as u8])?;
                     } else {
                         w.write_all(&[NEGINTSTART, n as u8 ^ 0xff])?;
+                        offset += 1;
                     }
                     (i.wrapping_sub(1).to_be_bytes())
                 };
                 w.write_all(&arr[SZ - n..])?;
 
-                Ok(VersionstampOffset::None { size: n as u32 + 1 })
+                Ok(offset)
             }
         }
 
@@ -470,7 +477,7 @@ macro_rules! impl_fx {
                 w.write_all(&[$code])?;
                 w.write_all(&bytes)?;
                 Ok(VersionstampOffset::None {
-                    size: std::mem::size_of::<$fx>() as u32 / 8 + 1,
+                    size: std::mem::size_of::<$fx>() as u32 + 1,
                 })
             }
         }
@@ -558,12 +565,14 @@ mod bigint {
             }
             let (sign, mut bytes) = self.to_bytes_be();
             let n = bytes.len();
+            let mut offset = VersionstampOffset::None { size: n as u32 + 1 };
             match sign {
                 Sign::Minus => {
                     if n <= MAX_SZ {
                         w.write_all(&[INTZERO - n as u8])?;
                     } else {
                         w.write_all(&[NEGINTSTART, bigint_n(n)? ^ 0xff])?;
+                        offset += 1;
                     }
                     invert(&mut bytes);
                     w.write_all(&bytes)?;
@@ -574,12 +583,13 @@ mod bigint {
                         w.write_all(&[INTZERO + n as u8])?;
                     } else {
                         w.write_all(&[POSINTEND, bigint_n(n)?])?;
+                        offset += 1;
                     }
                     w.write_all(&bytes)?;
                 }
-            };
+            }
 
-            Ok(VersionstampOffset::None { size: n as u32 + 1 })
+            Ok(offset)
         }
     }
 
@@ -626,14 +636,16 @@ mod bigint {
             }
             let bytes = self.to_bytes_be();
             let n = bytes.len();
+            let mut offset = VersionstampOffset::None { size: n as u32 + 1 };
             if n <= MAX_SZ {
                 w.write_all(&[INTZERO + n as u8])?;
             } else {
                 w.write_all(&[POSINTEND, bigint_n(n)?])?;
+                offset += 1;
             }
             w.write_all(&bytes)?;
 
-            Ok(VersionstampOffset::None { size: n as u32 + 1 })
+            Ok(offset)
         }
     }
 
@@ -995,10 +1007,11 @@ impl<'de> TupleUnpack<'de> for Element<'de> {
                 (input, Element::Uuid(v))
             }
             found => {
+                dbg!(Bytes::from(input));
                 return Err(PackError::BadCode {
                     found,
                     expected: None,
-                })
+                });
             }
         };
 
@@ -1025,7 +1038,11 @@ impl TuplePack for Versionstamp {
     ) -> io::Result<VersionstampOffset> {
         w.write_all(&[VERSIONSTAMP])?;
         w.write_all(self.as_bytes())?;
-        Ok(VersionstampOffset::One { offset: 1 })
+        if self.is_complete() {
+            Ok(VersionstampOffset::None { size: 1 + 12 })
+        } else {
+            Ok(VersionstampOffset::OneIncomplete { offset: 1 })
+        }
     }
 }