Merge branch 'master' into async-support-test

cs/src/core/Device/LocalStorageDevice.cs

@@ -47,15 +47,15 @@ public LocalStorageDevice(string filename,
 
         private void RecoverFiles()
         {
-            string[] comps = FileName.Split(Path.DirectorySeparatorChar);
-            string bareName = comps[comps.Length - 1];
-            string directory = System.IO.Path.GetDirectoryName(FileName);
-            DirectoryInfo di = new DirectoryInfo(directory);
+            FileInfo fi = new FileInfo(FileName); // may not exist
+            DirectoryInfo di = fi.Directory;
             if (!di.Exists) return;
+
+            string bareName = fi.Name;
+
             int prevSegmentId = -1;
             foreach (FileInfo item in di.GetFiles(bareName + "*"))
             {
-                // TODO(Tianyu): Depending on string parsing is bad. But what can one do when an entire cloud service API has no doc?
                 int segmentId = Int32.Parse(item.Name.Replace(bareName, "").Replace(".", ""));
                 if (segmentId != prevSegmentId + 1)
                 {
@@ -177,12 +177,12 @@ public override void RemoveSegmentAsync(int segment, AsyncCallback callback, IAs
             callback(result);
         }
 
-        // TODO(Tianyu): It may be somewhat inefficient to use the default async calls from the base class when the underlying
-        // method is inheritly synchronous. But just for delete (which is called infrequently and off the critical path) such
-        // inefficiency is probably negligible.
+        // It may be somewhat inefficient to use the default async calls from the base class when the underlying
+        // method is inherently synchronous. But just for delete (which is called infrequently and off the 
+        // critical path) such inefficiency is probably negligible.
 
         /// <summary>
-        /// 
+        /// Close device
         /// </summary>
         public override void Close()
         {

cs/src/core/Device/ManagedLocalStorageDevice.cs

@@ -42,15 +42,16 @@ public ManagedLocalStorageDevice(string filename, bool preallocateFile = false,
 
         private void RecoverFiles()
         {
-            string directory = System.IO.Path.GetDirectoryName(FileName);
-            DirectoryInfo di = new DirectoryInfo(directory);
+            FileInfo fi = new FileInfo(FileName); // may not exist
+            DirectoryInfo di = fi.Directory;
+            if (!di.Exists) return;
+
+            string bareName = fi.Name;
+
             int prevSegmentId = -1;
-            foreach (FileInfo item in di.GetFiles(FileName + "*"))
+            foreach (FileInfo item in di.GetFiles(bareName + "*"))
             {
-                Console.WriteLine(FileName);
-                // TODO(Tianyu): Depending on string parsing is bad. But what can one do when an entire cloud service API has no doc?
-                int segmentId = Int32.Parse(item.Name.Replace(FileName, "").Replace(".", ""));
-                Console.WriteLine(segmentId);
+                int segmentId = Int32.Parse(item.Name.Replace(bareName, "").Replace(".", ""));
                 if (segmentId != prevSegmentId + 1)
                 {
                     startSegment = segmentId;

cs/src/core/Device/ShardedStorageDevice.cs

@@ -18,14 +18,15 @@ interface IPartitionScheme
         /// used as unique identifiers for the shards.
         /// </summary>
         IList<IDevice> Devices { get; }
+
         /// <summary>
         /// Maps a range in the unified logical address space into a contiguous physical chunk on a shard's address space.
         /// Because the given range may be sharded across multiple devices, only the largest contiguous chunk starting from
         /// start address but smaller than end address is returned in shard, shardStartAddress, and shardEndAddress.
         /// </summary>
         /// <param name="startAddress">start address of the range to map in the logical address space</param>
         /// <param name="endAddress">end address of the range to map in the logical address space</param>
-        /// <param name="shard"> the shard (potentially part of) the given range resides in, given as index into <see cref="devices"/></param>
+        /// <param name="shard"> the shard (potentially part of) the given range resides in, given as index into <see cref="Devices"/></param>
         /// <param name="shardStartAddress"> start address translated into physical start address on the returned shard </param>
         /// <param name="shardEndAddress">
         /// physical address of the end of the part of the range on the returned shard. This is not necessarily a translation of the end address
@@ -95,7 +96,6 @@ public UniformPartitionScheme(long chunkSize, params IDevice[] devices) : this(c
         /// <returns></returns>
         public long MapRange(long startAddress, long endAddress, out int shard, out long shardStartAddress, out long shardEndAddress)
         {
-            // TODO(Tianyu): Can do bitshift magic for faster translation assuming chunk size is a multiple of 2
             long chunkId = startAddress / chunkSize;
             shard = (int)(chunkId % Devices.Count);
             shardStartAddress = chunkId / Devices.Count * chunkSize + startAddress % chunkSize;
@@ -120,8 +120,6 @@ public long MapRange(long startAddress, long endAddress, out int shard, out long
         /// <returns></returns>
         public long MapSectorSize(long sectorSize, int shard)
         {
-            // TODO(Tianyu): Is there an easier way to do this?
-            // TODO(Tianyu): Perform bit-shifting magic
             var numChunks = sectorSize / chunkSize;
             // ceiling of (a div b) is (a + b - 1) / b where div is mathematical division and / is integer division 
             return (numChunks + Devices.Count - 1) / Devices.Count * chunkSize;
@@ -220,14 +218,14 @@ public unsafe override void WriteAsync(IntPtr sourceAddress, int segmentId, ulon
                 // Because more than one device can return with an error, it is important that we remember the most recent error code we saw. (It is okay to only
                 // report one error out of many. It will be as if we failed on that error and cancelled all other reads, even though we issue reads in parallel and
                 // wait until all of them are complete in the implementation) 
-                // TODO(Tianyu): Can there be races on async result as we issue writes or reads in parallel?
+                // Can there be races on async result as we issue writes or reads in parallel?
                 partitions.Devices[shard].WriteAsync(IntPtr.Add(sourceAddress, (int)writeOffset),
                                                      segmentId,
                                                      (ulong)shardStartAddress,
                                                      (uint)(shardEndAddress - shardStartAddress),
                                                      (e, n, o) =>
                                                      {
-                                                         // TODO(Tianyu): It is incorrect to ignore o, as there might be a memory leak
+                                                         // TODO: Check if it is incorrect to ignore o
                                                          if (e != 0) aggregateErrorCode = e;
                                                          if (countdown.Signal())
                                                          {
@@ -244,7 +242,7 @@ public unsafe override void WriteAsync(IntPtr sourceAddress, int segmentId, ulon
                 currentWriteStart = newStart;
             }
 
-            // TODO(Tianyu): What do for the dumb overlapped wrapper...
+            // TODO: Check if overlapped wrapper is handled correctly
             if (countdown.Signal())
             {
                 Overlapped ov = new Overlapped(0, 0, IntPtr.Zero, asyncResult);
@@ -277,15 +275,14 @@ public unsafe override void ReadAsync(int segmentId, ulong sourceAddress, IntPtr
                 // Because more than one device can return with an error, it is important that we remember the most recent error code we saw. (It is okay to only
                 // report one error out of many. It will be as if we failed on that error and cancelled all other reads, even though we issue reads in parallel and
                 // wait until all of them are complete in the implementation) 
-                // TODO(Tianyu): Can there be races on async result as we issue writes or reads in parallel?
                 countdown.AddCount();
                 partitions.Devices[shard].ReadAsync(segmentId,
                                                     (ulong)shardStartAddress,
                                                     IntPtr.Add(destinationAddress, (int)writeOffset),
                                                     (uint)(shardEndAddress - shardStartAddress),
                                                     (e, n, o) =>
                                                     {
-                                                        // TODO(Tianyu): this is incorrect if returned "bytes" written is allowed to be less than requested like POSIX.
+                                                        // TODO: this is incorrect if returned "bytes" written is allowed to be less than requested like POSIX.
                                                         if (e != 0) aggregateErrorCode = e;
                                                         if (countdown.Signal())
                                                         {
@@ -302,7 +299,7 @@ public unsafe override void ReadAsync(int segmentId, ulong sourceAddress, IntPtr
                 currentReadStart = newStart;
             }
 
-            // TODO(Tianyu): What do for the dumb overlapped wrapper...
+            // TODO: Check handling of overlapped wrapper
             if (countdown.Signal())
             {
                 Overlapped ov = new Overlapped(0, 0, IntPtr.Zero, asyncResult);

cs/src/core/Device/StorageDeviceBase.cs

@@ -94,8 +94,6 @@ public StorageDeviceBase(string filename, uint sectorSize, long capacity)
         /// <param name="epoch"></param>
         public virtual void Initialize(long segmentSize, LightEpoch epoch = null)
         {
-            // TODO(Tianyu): Alternatively, we can adjust capacity based on the segment size: given a phsyical upper limit of capacity,
-            // we only make use of (Capacity / segmentSize * segmentSize) many bytes. 
             Debug.Assert(Capacity == -1 || Capacity % segmentSize == 0, "capacity must be a multiple of segment sizes");
             this.segmentSize = segmentSize;
             this.epoch = epoch;

cs/src/core/Device/TieredStorageDevice.cs

@@ -19,7 +19,6 @@ class TieredStorageDevice : StorageDeviceBase
         private readonly IList<IDevice> devices;
         private readonly int commitPoint;
 
-        // TODO(Tianyu): Not reasoning about what the sector size of a tiered storage should be when different tiers can have different sector sizes.
         /// <summary>
         /// Constructs a new TieredStorageDevice composed of the given devices.
         /// </summary>
@@ -32,11 +31,10 @@ class TieredStorageDevice : StorageDeviceBase
         /// List of devices to be used. The list should be given in order of hot to cold. Read is served from the
         /// device with smallest index in the list that has the requested data
         /// </param>
-        // TODO(Tianyu): Recovering from a tiered device is potentially difficult, because we also need to recover their respective ranges.
         public TieredStorageDevice(int commitPoint, IList<IDevice> devices) : base(ComputeFileString(devices, commitPoint), 512, ComputeCapacity(devices))
         {
             Debug.Assert(commitPoint >= 0 && commitPoint < devices.Count, "commit point is out of range");
-            // TODO(Tianyu): Should assert that passed in devices are not yet initialized. This is more challenging for recovering.
+
             this.devices = devices;
             this.commitPoint = commitPoint;
         }
@@ -57,8 +55,6 @@ public TieredStorageDevice(int commitPoint, params IDevice[] devices) : this(com
         {
         }
 
-
-        // TODO(Tianyu): Unclear whether this is the right design. Should we allow different tiers different segment sizes?
         public override void Initialize(long segmentSize, LightEpoch epoch)
         {
             base.Initialize(segmentSize, epoch);
@@ -90,7 +86,6 @@ public override unsafe void WriteAsync(IntPtr sourceAddress, int segmentId, ulon
         {
 
             int startTier = FindClosestDeviceContaining(segmentId);
-            // TODO(Tianyu): Can you ever initiate a write that is after the commit point? Given FASTER's model of a read-only region, this will probably never happen.
             Debug.Assert(startTier <= commitPoint, "Write should not elide the commit point");
 
             var countdown = new CountdownEvent(commitPoint + 1);  // number of devices to wait on
@@ -146,7 +141,6 @@ private static long ComputeCapacity(IList<IDevice> devices)
                 // Unless the last tier device has unspecified storage capacity, in which case the tiered storage also has unspecified capacity
                 if (device.Capacity == Devices.CAPACITY_UNSPECIFIED)
                 {
-                    // TODO(Tianyu): Is this assumption too strong?
                     Debug.Assert(device == devices[devices.Count - 1], "Only the last tier storage of a tiered storage device can have unspecified capacity");
                     return Devices.CAPACITY_UNSPECIFIED;
                 }
@@ -155,7 +149,6 @@ private static long ComputeCapacity(IList<IDevice> devices)
             return result;
         }
 
-        // TODO(Tianyu): Is the notion of file name still relevant in a tiered storage device?
         private static string ComputeFileString(IList<IDevice> devices, int commitPoint)
         {
             StringBuilder result = new StringBuilder();
@@ -177,7 +170,6 @@ private int FindClosestDeviceContaining(int segment)
             {
                 if (devices[i].StartSegment <= segment) return i;
             }
-            // TODO(Tianyu): This exception should never be triggered if we enforce that the last tier has unbounded storage.
             throw new ArgumentException("No such address exists");
         }
     }

cs/src/devices/AzureStorageDevice/AzureStorageDevice.cs

@@ -58,7 +58,6 @@ private void RecoverBlobs()
             int prevSegmentId = -1;
             foreach (IListBlobItem item in container.ListBlobs(blobName))
             {
-                // TODO(Tianyu): Depending on string parsing is bad. But what can one do when an entire cloud service API has no doc?
                 string[] parts = item.Uri.Segments;
                 int segmentId = Int32.Parse(parts[parts.Length - 1].Replace(blobName, ""));
                 if (segmentId != prevSegmentId + 1)

cs/src/devices/AzureStorageDevice/BlobEntry.cs

@@ -70,7 +70,6 @@ public void CreateAsync(long size, CloudPageBlob pageBlob)
                 }
                 catch (Exception e)
                 {
-                    // TODO(Tianyu): Can't really do better without knowing error behavior
                     Trace.TraceError(e.Message);
                 }
                 // At this point the blob is fully created. After this line all consequent writers will write immediately. We just

cs/test/BasicDiskFASTERTests.cs

@@ -58,7 +58,6 @@ public void TieredWriteRead()
         [Test]
         public void ShardedWriteRead()
         {
-            // TODO(Tianyu): Magic constant
             IDevice localDevice1 = Devices.CreateLogDevice(TestContext.CurrentContext.TestDirectory + "\\BasicDiskFASTERTests1.log", deleteOnClose: true, capacity: 1 << 30);
             IDevice localDevice2 = Devices.CreateLogDevice(TestContext.CurrentContext.TestDirectory + "\\BasicDiskFASTERTests2.log", deleteOnClose: true, capacity: 1 << 30);
             var device = new ShardedStorageDevice(new UniformPartitionScheme(512, localDevice1, localDevice2));