Traffic Server
  1. Traffic Server
  2. TS-745

Support ssd as a tiered cache component

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.3.5
    • Component/s: Cache
    • Labels:
      None

      Description

      A patch for supporting, not work well for a long time with --enable-debug

      1. TS-ssd.patch
        44 kB
        mohan_zl
      2. TS-ssd-2.patch
        13 kB
        mohan_zl
      3. ts-745.diff
        60 kB
        weijin
      4. 0001-TS-745-support-interim-caching-in-storage.patch
        63 kB
        Zhao Yongming
      5. 0001-TS-745-support-interim-caching-in-storage_rebased.patch
        63 kB
        Zhao Yongming

        Issue Links

          Activity

          Hide
          Leif Hedstrom added a comment -

          Closing this for now, but opening up a followup bug for v3.5.x

          Show
          Leif Hedstrom added a comment - Closing this for now, but opening up a followup bug for v3.5.x
          Hide
          ASF subversion and git services added a comment -

          Commit adaefd7893e89a18d6a45de0210dda70a8067138 in branch refs/heads/master from Zhao Yongming
          [ https://git-wip-us.apache.org/repos/asf?p=trafficserver.git;h=adaefd7 ]

          TS-745: update CHANGES

          Show
          ASF subversion and git services added a comment - Commit adaefd7893e89a18d6a45de0210dda70a8067138 in branch refs/heads/master from Zhao Yongming [ https://git-wip-us.apache.org/repos/asf?p=trafficserver.git;h=adaefd7 ] TS-745 : update CHANGES
          Hide
          ASF subversion and git services added a comment -

          Commit 8006abcbe740af4a78a7e4e918b4ddaff2c62f79 in branch refs/heads/master from weijin
          [ https://git-wip-us.apache.org/repos/asf?p=trafficserver.git;h=8006abc ]

          TS-745: support interim caching in storage

          this is one of the SSD project result, the current solution make a
          interim block cache layer between the storage and ram cache.

          two records directive added.
          proxy.config.cache.interim.storage: disk device(s) for the interim
          cache
          proxy.config.cache.interim.migrate_threshold: controls how many times
          one content should be migrate to the interim cache from the storage

          Signed-off-by: Zhao Yongming <ming.zym@gmail.com>

          Show
          ASF subversion and git services added a comment - Commit 8006abcbe740af4a78a7e4e918b4ddaff2c62f79 in branch refs/heads/master from weijin [ https://git-wip-us.apache.org/repos/asf?p=trafficserver.git;h=8006abc ] TS-745 : support interim caching in storage this is one of the SSD project result, the current solution make a interim block cache layer between the storage and ram cache. two records directive added. proxy.config.cache.interim.storage: disk device(s) for the interim cache proxy.config.cache.interim.migrate_threshold: controls how many times one content should be migrate to the interim cache from the storage Signed-off-by: Zhao Yongming <ming.zym@gmail.com>
          Hide
          Leif Hedstrom added a comment -

          Yep, good with me. So in the normal build, everything in the disk layout is as it was, right?

          Long term, it'd be nice to have this configurable at "run-time" instead, which makes it easier to test (off by default of course).

          Show
          Leif Hedstrom added a comment - Yep, good with me. So in the normal build, everything in the disk layout is as it was, right? Long term, it'd be nice to have this configurable at "run-time" instead, which makes it easier to test (off by default of course).
          Hide
          Zhao Yongming added a comment -

          this feature is disabled by configure options, by default. and the disk format change, in #DEFINED out if you do not enable in configure. I think that is fairly safe for common users. and for who enable this feature, he will aware of all the pins. and someone may work out the perfect solution for multi-tiered storage later, or we don't need it anymore if we find out that the slowest disk is SSD years later, we can safely revert then.

          is that ok?

          Show
          Zhao Yongming added a comment - this feature is disabled by configure options, by default. and the disk format change, in #DEFINED out if you do not enable in configure. I think that is fairly safe for common users. and for who enable this feature, he will aware of all the pins. and someone may work out the perfect solution for multi-tiered storage later, or we don't need it anymore if we find out that the slowest disk is SSD years later, we can safely revert then. is that ok?
          Hide
          Leif Hedstrom added a comment -

          Question: does this patch preserve the old cache/directory format when the feature is not enabled? I agree with John that doing these invasive changes, particularly changing the directory size, and the coupling between disks, is potentially bad. If however the cache is backward compatible with these features disabled, I'm ok with landing it for v3.4.

          Show
          Leif Hedstrom added a comment - Question: does this patch preserve the old cache/directory format when the feature is not enabled? I agree with John that doing these invasive changes, particularly changing the directory size, and the coupling between disks, is potentially bad. If however the cache is backward compatible with these features disabled, I'm ok with landing it for v3.4.
          Hide
          Zhao Yongming added a comment -

          minor change in stat names. ready for commit.

          updated the design and usage docs on https://cwiki.apache.org/confluence/display/TS/SSDSupport

          Show
          Zhao Yongming added a comment - minor change in stat names. ready for commit. updated the design and usage docs on https://cwiki.apache.org/confluence/display/TS/SSDSupport
          Hide
          Zhao Yongming added a comment -

          rebased on current master

          Show
          Zhao Yongming added a comment - rebased on current master
          Hide
          John Plevyak added a comment -

          Humm... let me read over the code. An SSD layer is necessary at this point, and if this is ephemeral, I am sure we can find a clean integration.

          thanx!

          Show
          John Plevyak added a comment - Humm... let me read over the code. An SSD layer is necessary at this point, and if this is ephemeral, I am sure we can find a clean integration. thanx!
          Hide
          weijin added a comment -

          the ssd is treated as the (block) cache of hdd disk, and the ssd dir will be cleared when you start (restart) ats, so it also can unplug any hdd disk and change it back.

          I admit this patch is pretty invasive and make cache more complicated, and I appropriate that someone can give us a more general and elegant design of tired cache solution.

          Show
          weijin added a comment - the ssd is treated as the (block) cache of hdd disk, and the ssd dir will be cleared when you start (restart) ats, so it also can unplug any hdd disk and change it back. I admit this patch is pretty invasive and make cache more complicated, and I appropriate that someone can give us a more general and elegant design of tired cache solution.
          Hide
          John Plevyak added a comment -

          I think the idea of stealing bits from the directory which are hard coded to point off device (off the hard disk which the directory is a part of) is a huge design departure and a problem. When the cache was first built, it was limited to 8GB disks which seemed HUGE. For Apache I extended it to .5PB as by then 8GB was far too small. Currently disks are at 4TB and this patch would decrease the limit from .5PB to 32TB which gives us only a few years headroom, not a good idea. Furthermore, the current design let's you unplug any cache disk from any machine, move it to another machine and have your cache back. This change stores SSD information in the HDD directory! why? Changing the configuration, a disk or machine failure, etc. invalidates that information corrupting the cache. Why not store that information in a side structure and either store it only in memory only or on the SSD?

          The idea of storing the SSD configuration in a string in records.config is also a bad idea.

          Overall, a stacked cache seems like a better idea or a minimally invasive extension would be great. This patch is pretty invasive, duplicates code and generally touches many bits of the code. The ram cache for example uses no bits in the HDD directory and only a couple entry points at well defined places (insert, lookup and delete/invalidate).

          This patch looks to incur more technical depth at a time when I think we would like to decrease the technical debt. For example, it would be nice to have more smaller locks, move the HTTP support out of the core via a well defined interface, add layering, etc. Adding yet another set of core code paths is going to make those changes harder.

          my 2 cents.

          Show
          John Plevyak added a comment - I think the idea of stealing bits from the directory which are hard coded to point off device (off the hard disk which the directory is a part of) is a huge design departure and a problem. When the cache was first built, it was limited to 8GB disks which seemed HUGE. For Apache I extended it to .5PB as by then 8GB was far too small. Currently disks are at 4TB and this patch would decrease the limit from .5PB to 32TB which gives us only a few years headroom, not a good idea. Furthermore, the current design let's you unplug any cache disk from any machine, move it to another machine and have your cache back. This change stores SSD information in the HDD directory! why? Changing the configuration, a disk or machine failure, etc. invalidates that information corrupting the cache. Why not store that information in a side structure and either store it only in memory only or on the SSD? The idea of storing the SSD configuration in a string in records.config is also a bad idea. Overall, a stacked cache seems like a better idea or a minimally invasive extension would be great. This patch is pretty invasive, duplicates code and generally touches many bits of the code. The ram cache for example uses no bits in the HDD directory and only a couple entry points at well defined places (insert, lookup and delete/invalidate). This patch looks to incur more technical depth at a time when I think we would like to decrease the technical debt. For example, it would be nice to have more smaller locks, move the HTTP support out of the core via a well defined interface, add layering, etc. Adding yet another set of core code paths is going to make those changes harder. my 2 cents.
          Hide
          Zhao Yongming added a comment -

          yeah, I'd like that too, then we don't have to enable that by configure

          Show
          Zhao Yongming added a comment - yeah, I'd like that too, then we don't have to enable that by configure
          Hide
          Leif Hedstrom added a comment -

          One more question: This feature, if I understand it, changes the cache and directory structure fairly substantially. Presumably this invalidates the cache at a minimum, and potentially (long term) limits how people use the cache (e.g. max volume sizes etc.).

          Would it be possible / reasonable to make it such that unless there is a "tiered" cache layout/setup, the cache (and dirents) stay compatible with what we have now? This would make transition, and refinements, of this feature much less intrusive. Someone who doesn't enable it, won't notice anything, and the cache would just continue to work as it was (without losing any data).

          Thoughts?

          Show
          Leif Hedstrom added a comment - One more question: This feature, if I understand it, changes the cache and directory structure fairly substantially. Presumably this invalidates the cache at a minimum, and potentially (long term) limits how people use the cache (e.g. max volume sizes etc.). Would it be possible / reasonable to make it such that unless there is a "tiered" cache layout/setup, the cache (and dirents) stay compatible with what we have now? This would make transition, and refinements, of this feature much less intrusive. Someone who doesn't enable it, won't notice anything, and the cache would just continue to work as it was (without losing any data). Thoughts?
          Hide
          Zhao Yongming added a comment -

          new version of the code cleanup and rebase, changed all the tiered & SSD words into Interim, for we have made a interim block cache for the storage. also changed the records configs. we can keep the backword compatible with some small patch.

          Show
          Zhao Yongming added a comment - new version of the code cleanup and rebase, changed all the tiered & SSD words into Interim, for we have made a interim block cache for the storage. also changed the records configs. we can keep the backword compatible with some small patch.
          Hide
          James Peach added a comment -

          FWIW, I'm fine with this plan. We need to ensure that the various features co-operate in a sensible fashion, but I'm also OK with supporting Taobao's need for compatibility with the records.config options.

          Show
          James Peach added a comment - FWIW, I'm fine with this plan. We need to ensure that the various features co-operate in a sensible fashion, but I'm also OK with supporting Taobao's need for compatibility with the records.config options.
          Hide
          Leif Hedstrom added a comment -

          Exactly, that's why we need to figure this out together, and not do one off "hacks" . Having both TS-745 and TS-1728 seems fine with me, but they need to cooperate together nicely (even if it means one takes priority over the other).

          Show
          Leif Hedstrom added a comment - Exactly, that's why we need to figure this out together, and not do one off "hacks" . Having both TS-745 and TS-1728 seems fine with me, but they need to cooperate together nicely (even if it means one takes priority over the other).
          Hide
          Zhao Yongming added a comment -

          well, I can not tell how to merge the config with TS-1728, that is working on the volume assign tricky, but we are working on the block level. I really like to get some idea how the performance compared in Comcast.

          in common case, if you can identify what is the good way to use the disks, you can make a good usage by assign the SSDs to the site that really need it. in real production, you'd need to figure that out, and monitor it and adapt when they changed, sounds not that good for complex site

          question is, when we merge both patches, how to tell the system, we just care of some disks and assign some tiered storage for them, but not other disks? sounds some complex matrix, or some more coding needed

          /dev/sda tier=1 volume=2
          /dev/sdb tier=1 volume=1
          /dev/sdc tier=2 volume=1
          /dev/sdd tier=2 volume=3

          Show
          Zhao Yongming added a comment - well, I can not tell how to merge the config with TS-1728 , that is working on the volume assign tricky, but we are working on the block level. I really like to get some idea how the performance compared in Comcast. in common case, if you can identify what is the good way to use the disks, you can make a good usage by assign the SSDs to the site that really need it. in real production, you'd need to figure that out, and monitor it and adapt when they changed, sounds not that good for complex site question is, when we merge both patches, how to tell the system, we just care of some disks and assign some tiered storage for them, but not other disks? sounds some complex matrix, or some more coding needed /dev/sda tier=1 volume=2 /dev/sdb tier=1 volume=1 /dev/sdc tier=2 volume=1 /dev/sdd tier=2 volume=3
          Hide
          Leif Hedstrom added a comment - - edited

          We had a long, heated discussion about this, so capturing some of this in this email.

          First, we all agree that this should be a "Tiered Cache", and not specific to SSD. So code and configs etc. should properly reflect that.

          Secondly, I obviously dislike the idea of storing some disk configs in storage.config, and some in records.config. I understand making this change is a big deal for Taobao, so I'm offering a compromise:

          1) We add support for storage.config, with a new tag similar to the tag TS-1728 adds for volume identification. Maybe something like

          /dev/sda tier=1
          /dev/sdb tier=1
          /dev/sdc tier=2
          /dev/sdd tier=2
          

          Where tier=1 is "SSD" (or whatever fast storage device you have, what currently gets configured in records.config) and tier=2 is all other disks (what's currently in storage.config).

          2) In addition, we keep the records.config setting in mgmt/RecordsConfig.cc, and make it "populate" the tier=1 disks. This option will not be documented, or added to records.config.default.in. It's purely there to allow Taobao to merge with upstream and not having to maintain a fork. Long term, I'd imagine this config goes away, as we refactor all the configuraitons into a unified "VirtualHost" conceptual config.

          James and I (and hopefully John too) will continue reviewing this complicated piece of code in the next week or two. The goal is to land this for v3.3.3, which is Mid-May. We do need to make sure this also doesn't conflict with TS-1728, which also should be landed for v3.3.3.

          Feedback please.

          Show
          Leif Hedstrom added a comment - - edited We had a long, heated discussion about this, so capturing some of this in this email. First, we all agree that this should be a "Tiered Cache", and not specific to SSD. So code and configs etc. should properly reflect that. Secondly, I obviously dislike the idea of storing some disk configs in storage.config, and some in records.config. I understand making this change is a big deal for Taobao, so I'm offering a compromise: 1) We add support for storage.config, with a new tag similar to the tag TS-1728 adds for volume identification. Maybe something like /dev/sda tier=1 /dev/sdb tier=1 /dev/sdc tier=2 /dev/sdd tier=2 Where tier=1 is "SSD" (or whatever fast storage device you have, what currently gets configured in records.config) and tier=2 is all other disks (what's currently in storage.config). 2) In addition, we keep the records.config setting in mgmt/RecordsConfig.cc, and make it "populate" the tier=1 disks. This option will not be documented, or added to records.config.default.in. It's purely there to allow Taobao to merge with upstream and not having to maintain a fork. Long term, I'd imagine this config goes away, as we refactor all the configuraitons into a unified "VirtualHost" conceptual config. James and I (and hopefully John too) will continue reviewing this complicated piece of code in the next week or two. The goal is to land this for v3.3.3, which is Mid-May. We do need to make sure this also doesn't conflict with TS-1728 , which also should be landed for v3.3.3. Feedback please.
          Hide
          James Peach added a comment -

          We need to resolve how TS-745 and TS-1728 work together and should be configured in a sensible way.

          Show
          James Peach added a comment - We need to resolve how TS-745 and TS-1728 work together and should be configured in a sensible way.
          Hide
          Leif Hedstrom added a comment -

          Couple of things with the latest patch:

          1) Configuration for this does most certainly not belong in records.config. It should be integrated with storage.config (and perhaps hosting.config and volumes.config as well).

          2) I dislike that we use the variable/method/config names with "SSD" in them. The configure option is to enable tiered storage, and I think the code and configs should reflect this as well.

          Show
          Leif Hedstrom added a comment - Couple of things with the latest patch: 1) Configuration for this does most certainly not belong in records.config. It should be integrated with storage.config (and perhaps hosting.config and volumes.config as well). 2) I dislike that we use the variable/method/config names with "SSD" in them. The configure option is to enable tiered storage, and I think the code and configs should reflect this as well.
          Hide
          weijin added a comment -

          make the ssd as cache of sas disks.

          steal 4 bits from the dir. 1 bit to identify the dir is for ssd or sas; 3 bits to index which ssd vol.

          Migrate to ssd based on fragment rather than document.

          need more reviews and comments

          Show
          weijin added a comment - make the ssd as cache of sas disks. steal 4 bits from the dir. 1 bit to identify the dir is for ssd or sas; 3 bits to index which ssd vol. Migrate to ssd based on fragment rather than document. need more reviews and comments
          Hide
          Zhao Yongming added a comment -

          after discuss on the IRC, we'd start to merge and rebase the codes onto git master, and prepare for code review.

          Show
          Zhao Yongming added a comment - after discuss on the IRC, we'd start to merge and rebase the codes onto git master, and prepare for code review.
          Hide
          Zhao Yongming added a comment -

          woo, that will be a tough task. and Wei Jin had make another implement on tiered storage, which is based on 3.0.x tree, codes on: https://gitorious.org/trafficserver/taobao/commits/new_tbtrunk_ssd, without docs as always.

          the tiered storage does not work well with multi-volume config for now.

          I am still not sure we should move forward to merge it in our office tree or not, although it works perfect for me. in my hardware spec, mem:ssd:sas=1:10:100.

          if you guys think the tiered storage is a right way to go, we will merge it into the git master.

          thanks

          Show
          Zhao Yongming added a comment - woo, that will be a tough task. and Wei Jin had make another implement on tiered storage, which is based on 3.0.x tree, codes on: https://gitorious.org/trafficserver/taobao/commits/new_tbtrunk_ssd , without docs as always. the tiered storage does not work well with multi-volume config for now. I am still not sure we should move forward to merge it in our office tree or not, although it works perfect for me. in my hardware spec, mem:ssd:sas=1:10:100. if you guys think the tiered storage is a right way to go, we will merge it into the git master. thanks
          Hide
          Igor Galić added a comment -

          Can you please rebase this branch against the current trunk?

          Show
          Igor Galić added a comment - Can you please rebase this branch against the current trunk?
          Hide
          Leif Hedstrom added a comment -

          Moving this out to 3.3.0, please move back to 3.1.4 if this will be worked on soon. Also, take a look at what can be closed here please!

          Show
          Leif Hedstrom added a comment - Moving this out to 3.3.0, please move back to 3.1.4 if this will be worked on soon . Also, take a look at what can be closed here please!
          Hide
          Leif Hedstrom added a comment -

          Moved to 3.1.4, please move bugs back to 3.1.3, which you will work on in the next 2 weeks.

          Show
          Leif Hedstrom added a comment - Moved to 3.1.4, please move bugs back to 3.1.3, which you will work on in the next 2 weeks.
          Hide
          Leif Hedstrom added a comment -

          Moving these to 3.1.2 for now. please move back if they will be worked on asap for 3.1.1.

          Show
          Leif Hedstrom added a comment - Moving these to 3.1.2 for now. please move back if they will be worked on asap for 3.1.1.
          Hide
          mohan_zl added a comment -

          TS-ssd-2.patch correct the log stats, and add a "proxy.config.cache.ram_cache.ssd_percent" variable for ssd. For example, if user has 8G cache, and want ssd use 4G, just set this variable to 50.

          Show
          mohan_zl added a comment - TS-ssd-2.patch correct the log stats, and add a "proxy.config.cache.ram_cache.ssd_percent" variable for ssd. For example, if user has 8G cache, and want ssd use 4G, just set this variable to 50.
          Hide
          Zhao Yongming added a comment -

          done commit the TS-ssd.patch into branch ssd.

          Show
          Zhao Yongming added a comment - done commit the TS-ssd.patch into branch ssd.
          Hide
          mohan_zl added a comment -

          I have been testing this patch for some days, and now it looks stable.
          1. The created config file storage_ssd.config has the same syntax with the storage.config file, users can use one or more ssd, if there is no storage_ssd.config file or the file have no content, then ssd is disabled.
          2. I disable the multi-fragment feature in this version, ssd can save file whose size is between [0, proxy.config.cache.target_fragment_size). In the next step i will consider to add multi-fragment feature.

          Show
          mohan_zl added a comment - I have been testing this patch for some days, and now it looks stable. 1. The created config file storage_ssd.config has the same syntax with the storage.config file, users can use one or more ssd, if there is no storage_ssd.config file or the file have no content, then ssd is disabled. 2. I disable the multi-fragment feature in this version, ssd can save file whose size is between [0, proxy.config.cache.target_fragment_size). In the next step i will consider to add multi-fragment feature.
          Hide
          mohan_zl added a comment -

          Latest ssd patch.

          Show
          mohan_zl added a comment - Latest ssd patch.
          Hide
          mohan_zl added a comment -

          I'm considering the multi-fragment problem, i wanna know besides large file examples, when will use multi fragments?

          Show
          mohan_zl added a comment - I'm considering the multi-fragment problem, i wanna know besides large file examples, when will use multi fragments?
          Hide
          mohan_zl added a comment -

          My boss have give me another passport to visit http://codereview.appspot.com/. John, i did not search the traffic server results in this site.

          Show
          mohan_zl added a comment - My boss have give me another passport to visit http://codereview.appspot.com/ . John, i did not search the traffic server results in this site.
          Hide
          mohan_zl added a comment -

          In my thoughts, if we use the same CacheVC to write after calling back the user, we can do it before free_CacheVC, but when i did this, i could not pass the function test.

          Show
          mohan_zl added a comment - In my thoughts, if we use the same CacheVC to write after calling back the user, we can do it before free_CacheVC, but when i did this, i could not pass the function test.
          Hide
          mohan_zl added a comment -

          I'm sorry John, maybe we could not use http://codereview.appspot.com/ for detailed reviews, because i am living in a magical country like Korea, i couldn't open that website...
          Current changes follow your advices:
          (1) Now the new patch delete 146 lines, and add 842 lines. I'm trying to avoid code duplicate
          (2) Now the patch support multi SSD, the config file is storage_ssd.config file, and the syntax to add file is the same as storage.config
          (3)(4)(5) I now use xmalloc to store and xfree to delete the docs. I don't understand how to use various allocators to malloc and free the docs. Besides, now i also use another CacheVC to write the docs to ssd, because i have no idea to write after calling back the user, so (4) and (5) is not done also.

          Show
          mohan_zl added a comment - I'm sorry John, maybe we could not use http://codereview.appspot.com/ for detailed reviews, because i am living in a magical country like Korea, i couldn't open that website... Current changes follow your advices: (1) Now the new patch delete 146 lines, and add 842 lines. I'm trying to avoid code duplicate (2) Now the patch support multi SSD, the config file is storage_ssd.config file, and the syntax to add file is the same as storage.config (3)(4)(5) I now use xmalloc to store and xfree to delete the docs. I don't understand how to use various allocators to malloc and free the docs. Besides, now i also use another CacheVC to write the docs to ssd, because i have no idea to write after calling back the user, so (4) and (5) is not done also.
          Hide
          mohan_zl added a comment -

          +1 on a branch. I am busy on preparing ts for online use these days, sorry for looking at this so late.

          Show
          mohan_zl added a comment - +1 on a branch. I am busy on preparing ts for online use these days, sorry for looking at this so late.
          Hide
          Leif Hedstrom added a comment -

          +1 on a branch for this, and we'll land it back onto v3.1.

          Show
          Leif Hedstrom added a comment - +1 on a branch for this, and we'll land it back onto v3.1.
          Hide
          John Plevyak added a comment -

          We should make a branch for this. Then we could collaborate. I would also like to simplify locking in the cache by getting rid of the TryLocks for the partition. This would make it easier to write the SSD code to handle multiple SSDs I think.

          What do you think mohan_zl?

          Show
          John Plevyak added a comment - We should make a branch for this. Then we could collaborate. I would also like to simplify locking in the cache by getting rid of the TryLocks for the partition. This would make it easier to write the SSD code to handle multiple SSDs I think. What do you think mohan_zl?
          Hide
          John Plevyak added a comment -

          Overall several comments:

          1) Once we get it working we need to reorganize the patch so that we avoid code duplication.
          2) The limitation of a single SSD is probably overly restrictive. SSDs are relatively inexpensive now and it is very likely that folks will want to have 1 per disk or better yet some combination of SSD and SATA.
          3) The code uses xmalloc to store docs and creates another CacheVC to do the write in the background. We would be using the various allocators rather than xmalloc and we should be adding states rather than creating another CacheVC. The write can occur after calling back the user, so there will be no additional latency.
          4) The code doesn't seem to have provision for handling multi-fragment documents. There are some tradeoffs there, but in any case the issues needs to be considered. Handling of multi-fragment documents requires tighter control over when the write is done and if and when it is successful. Again, this would indicate we should be doing the write on the same CacheVC in additional states.
          5) The code needs to deal collisions after the initial directory is looked up. Again, this would be easier if the SSD code was operating in the same CacheVC.

          Please think about these comments. Let's use http://codereview.appspot.com/ for detailed reviews. I have already added traffic server as a repository. I have imported the patch as well, but I think we still have some design discussion to do before we can get into the details.

          Show
          John Plevyak added a comment - Overall several comments: 1) Once we get it working we need to reorganize the patch so that we avoid code duplication. 2) The limitation of a single SSD is probably overly restrictive. SSDs are relatively inexpensive now and it is very likely that folks will want to have 1 per disk or better yet some combination of SSD and SATA. 3) The code uses xmalloc to store docs and creates another CacheVC to do the write in the background. We would be using the various allocators rather than xmalloc and we should be adding states rather than creating another CacheVC. The write can occur after calling back the user, so there will be no additional latency. 4) The code doesn't seem to have provision for handling multi-fragment documents. There are some tradeoffs there, but in any case the issues needs to be considered. Handling of multi-fragment documents requires tighter control over when the write is done and if and when it is successful. Again, this would indicate we should be doing the write on the same CacheVC in additional states. 5) The code needs to deal collisions after the initial directory is looked up. Again, this would be easier if the SSD code was operating in the same CacheVC. Please think about these comments. Let's use http://codereview.appspot.com/ for detailed reviews. I have already added traffic server as a repository. I have imported the patch as well, but I think we still have some design discussion to do before we can get into the details.
          Hide
          mohan_zl added a comment -

          Add missing file

          Show
          mohan_zl added a comment - Add missing file
          Hide
          mohan_zl added a comment -

          The demand is described in https://cwiki.apache.org/TS/ssdsupport.html, now i write a simple patch for supporting ssd. This is obviously a temporary scheme, and now it only supports one ssd plus several sata.

          Show
          mohan_zl added a comment - The demand is described in https://cwiki.apache.org/TS/ssdsupport.html , now i write a simple patch for supporting ssd. This is obviously a temporary scheme, and now it only supports one ssd plus several sata.

            People

            • Assignee:
              weijin
              Reporter:
              mohan_zl
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development