Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Craft More Intricate Config Files #136

Open
Tracked by #3
ykaravas opened this issue Jun 21, 2022 · 6 comments · May be fixed by #138
Open
Tracked by #3

Craft More Intricate Config Files #136

ykaravas opened this issue Jun 21, 2022 · 6 comments · May be fixed by #138

Comments

@ykaravas
Copy link

This issue references a task in #3, Test Coverage / Quality Umbrella; specifically, related to the fourteenth issue down the list which reads:

" Craft a config that can be used as a basis for most tests"

@ykaravas
Copy link
Author

ykaravas commented Jun 21, 2022

@HalosGhost I would like to take a crack at the task that reads " Craft a config that can be used as a basis for most tests". I think this will be helpful in furthering my understanding of the code and will obviously be useful for testing etc... I was wondering if you could elaborate a bit on what is meant by "as a basis for more tests"?

  • Are we looking for larger and more complex config files?

  • Are we looking for config files that will test specific scenarios?

  • Should these new config file(s) target specific integration tests?

  • Also, what do you think about creating some sort of simple tool (maybe in the tools folder or tests folder) that could either be a simple C++ program or a bash script that could autogenerate a large config file from a few parameters? For example, one could put number of wallets, shards, atomizers, sentinels etc.... Eventually, we could have parameters like average transactions per second and maybe even parameters that could be something like probability of failure for each type of component that could, in an integration test that would ingest these, simulate such failures and such volumes of transactions and many more dynamics as well I'm sure. I am not very familiar with the existing testing framework or the benchmarking tools you have used but let me know if you think this sounds useful. I suppose, however, I am thinking about this mainly, in a non-docker context. But, I can envision an integration test that could parse the autogenerated configuration file, use docker-cli spin up all the docker containers required in the proper network and issue commands such as transactions, minting and simulating outages based on parameters in the file.

@ykaravas
Copy link
Author

Regarding your comment as to whether or not it would conflict with #81 i think not necessarily. I envision the configuration generation, and subsequent possibility of more complicated integration tests, to exceed the ability of what could be done with feature flags. I may be wrong though

@ykaravas
Copy link
Author

ykaravas commented Jun 21, 2022

@HalosGhost I have made some good progress on this on a branch. I can create a draft pull request if you would like to take a look. I do have a few questions on top of the above. I am just curious why in the 2pc architecture configuration files, coordinators and shards have an extra "_count" configuration (which i believe corresponds to nodes since they are raft replicated) but the Atomizer does not in the atomizer architecture and that is also raft duplicated. Also, when i make the count for the coordinators or shards higher than 1, there seem to be issues with the tests; not sure if this is by design or it is simply messing with the output of the test and rendering it to fail.

Also, what exactly is the purpose of the shard_start and shard_end? I know they are IDs of some sort? But does the end - start equal how many shards there are in that particular shard cluster?

Also, i tried to recycle util/common/config when writing the code i have written, however, it did not exactly fully fit in the with configuration generation as i have done it. I do know, however, when it comes to expanding the autogeneration to be more complicated (aka to create more intricate scenarios as i envision it can), the stuff in config.hpp will be useful. However, i foresee needing to extend it, yet it is not a class. I was wondering if it would be acceptable to turn it into one so i could create a class that inherits from it that can be used in when ingesting the more intricate, autogenerated configuration files?

Finally, what exactly were the following configuration parameters meant to do:

  • loadgen_sendtx_output_count
  • loadgen_sendtx_input_count,
  • loadgen_invalid_tx_rate
  • loadgen_fixed_tx_rate

@HalosGhost
Copy link
Collaborator

@HalosGhost I have made some good progress on this on a branch. I can create a draft pull request if you would like to take a look.

I'm always happy to take a look at code (even if it's not quite ready for merge)! However, my backlog is growing, so I'll warn that it may be a little bit before I can review it.

I am just curious why in the 2pc architecture configuration files, coordinators and shards have an extra "_count" configuration[…]

<component>_count is how you tell the config parser to know how many components it should attempt to parse the config for (and is not specific to 2pc; in /config.cfg.sample, you can see these defined for the atomizer architecture). So, if you set shard_count to 5, but only provide configurations for shard0_* and shard1_*, that will cause issues (because the parser will be looking for shard{2,3,4}_* as well).

Also, what exactly is the purpose of the shard_start and shard_end?

These configurations declare what portion of the UHS the specific shard is responsible for. As background: the UHS is partitioned based on the first byte of UHS IDs. Shards, then, are responsible for some subset of the possible values of the first byte of UHS IDs. In the configuration, you specify what subset (“shard ranges”) the shard is responsible for (by specifying the decimal values of the ranges it should cover). In the sample configuration I mentioned above, there's a single shard responsible for the full range ([0, 255]), but it's entirely valid (and desirable) to have multiple shards which cover different (and potentially overlapping) ranges (so long as the entire [0, 255] is covered)

However, i foresee needing to extend [config.hpp], yet it is not a class. I was wondering if it would be acceptable to turn it into one so i could create a class that inherits from it that can be used in when ingesting the more intricate, autogenerated configuration files?

That's an interesting idea and I don't inherently have any problem with it (though I appreciate that the configuration we currently provide is rather POD); I think with that level of change, it might make sense to split the config-generation into a separate PR.

Finally, what exactly were the following configuration parameters meant to do:
* loadgen_sendtx_output_count
* loadgen_sendtx_input_count,
* loadgen_invalid_tx_rate
* loadgen_fixed_tx_rate

These are parameters for configuring the load-generators (a special component which is used with the test controller for large-scale benchmarks; it is not part of the transaction processor, but is essentially an orchestrator for wallets) to make large-scale benchmarks more easily-controllable. With that background, it might be more obvious what each does (how many outputs should load-generators include in each transaction, how many inputs, etc.).

@ykaravas
Copy link
Author

@HalosGhost Ok. I will create a draft merge request. Honestly, at this point, it actually may even be ready for a merge. I have the basics plus some, of a configuration generation schema up and running. And as you said, it may be wise to possibly merge it in before it gets too cumbersome (with a possible refactoring of cbdc::config. And no worries! I am sure you are very busy.

Thank you for the background info on my questions!

@ykaravas
Copy link
Author

@HalosGhost With regards to your response regarding the _count", i am aware of the atomizer_count, shard_count etc... However, if you look at the 2pc ones, for example, opencbdc-tx/tests/integration/integration_tests_2pc.cfg, you will see that there is a shard_count (which i understand) and a coordinator_count (which also makes sense). There are also, however, shard0_count and coordinator0_count in there. I thought these might be redundant at first but after removing them or even changing them it seems they are essential. So i am just curious what is the difference between the general shard_count, for example, and the shard0_count?

And ah ok; this is good to know. So the shard_start to shard_end can only be between 0 and 255 (inclusive). Thank you!

@ykaravas ykaravas linked a pull request Jun 22, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants