search

ECP-VeloC / VELOC

Very-Low Overhead Checkpointing System
http://veloc.rtfd.io
MIT License
53 stars 22 forks source link

Allow all AXL transfer types in config file #18

Closed tonyhutter closed 5 years ago

tonyhutter commented 5 years ago

Previously, VELOC would only allow axl_type = AXL_XFER_SYNC in the config file. This patch allows the user to specify any AXL transfer type. It defaults to AXL_XFER_SYNC if the transfer type is not specified.

bnicolae commented 5 years ago

@tonyhutter your patch is failing the basic test (double free corruption, maybe because of bump to AXL v2.0?). Please make sure you are testing your code before submitting a pull request. Also, the code itself in transfer_module seems to do shady stuff like allocating an array of strings of length AXL_XFER_BEST? This is supposed to be a constant corresponding to an AXL transfer type, not the number of AXL transfer types, right? I would use a std::map<string, int> (so you can directly query what id corresponds to what string instead of a for loop)

bnicolae commented 5 years ago

@tonyhutter in case you didn't notice, we have a Travis CI for VELOC: https://travis-ci.org/ECP-VeloC/VELOC/builds/512274759

You can use the same approach to test your code locally (meaning take a look at .travis.yml in the root of VELOC)

tonyhutter commented 5 years ago

@bnicolae I did not hit the veloc-backend error when I was testing it: *** Error in `/home/travis/deploy/bin/veloc-backend': double free or corruption (fasttop): 0x00007f3218001200 *** I'll have to look into it. Thanks for the tip on std::map<string, int>; I'll update axl_type_str_to_type() to use it.

bnicolae commented 5 years ago

Tony, don't worry about the std::map. I am just about to check in the new code.

bnicolae commented 5 years ago

@tonyhutter I've changed the VELOC code accordingly. Please test against the new master branch.

bnicolae commented 5 years ago

Reverted to original master to avoid merge conflicts. @tonyhutter please go ahead and address these issues on your side.

tonyhutter commented 5 years ago

@bnicolae I just pushed an updated version of this patch. Please take another look.