boundary / folsom

Expose Erlang Events and Metrics
Apache License 2.0
585 stars 166 forks source link

store samples in ets for types none and uniform #11

Closed nygge closed 12 years ago

nygge commented 12 years ago

To avoid copying the complete datasets from and to ets for each update, the samples are now stored in its own ets table.

joewilliams commented 12 years ago

Any reason that I shouldn't go ahead and switch the exdec sample to ets as well?

nygge commented 12 years ago

I don´t think so. the only reason that I haven´t done it is that I never got to it.

On Thu, Dec 1, 2011 at 12:03 AM, Joe Williams reply@reply.github.com wrote:

Any reason that I shouldn't go ahead and switch the exdec sample to ets as well?


Reply to this email directly or view it on GitHub: https://github.com/boundary/folsom/pull/11#issuecomment-2971321

nygge commented 12 years ago

And just to be clear, these are all the patches I have at this time. And I am not able to do anything more on this in the near future.

/Anders

On Thu, Dec 1, 2011 at 12:56 AM, Anders Nygren anders.nygren@gmail.com wrote:

I don´t think so. the only reason that I haven´t done it is that I never got to it.

On Thu, Dec 1, 2011 at 12:03 AM, Joe Williams reply@reply.github.com wrote:

Any reason that I shouldn't go ahead and switch the exdec sample to ets as well?


Reply to this email directly or view it on GitHub: https://github.com/boundary/folsom/pull/11#issuecomment-2971321

joewilliams commented 12 years ago

No problem, I am thankful for the patches you have had time to contribute thus far. Hopefully I will have time to continue the work. :)

-Joe

Name: Joseph A. Williams Email: williams.joe@gmail.com

On Thursday, December 1, 2011 at 7:44 AM, Anders Nygren wrote:

And just to be clear, these are all the patches I have at this time. And I am not able to do anything more on this in the near future.

/Anders

On Thu, Dec 1, 2011 at 12:56 AM, Anders Nygren <anders.nygren@gmail.com (mailto:anders.nygren@gmail.com)> wrote:

I don´t think so. the only reason that I haven´t done it is that I never got to it.

On Thu, Dec 1, 2011 at 12:03 AM, Joe Williams <reply@reply.github.com (mailto:reply@reply.github.com)> wrote:

Any reason that I shouldn't go ahead and switch the exdec sample to ets as well?


Reply to this email directly or view it on GitHub: https://github.com/boundary/folsom/pull/11#issuecomment-2971321


Reply to this email directly or view it on GitHub: https://github.com/boundary/folsom/pull/11#issuecomment-2976163

joewilliams commented 12 years ago

Anders, you mentioned a patch for the random seed issue, I don't see that in these pull requests, is that something you wanted me to merge?

nygge commented 12 years ago

It is already included for uniform samples. In the #uniform record is a field seed that gets initialized to now(), see folsom.hrl. And in folsom_sample_uniform random:uniform_s/2 is used. A similar thing could/should be done for folsom_sample_exdec.

Remember that there is also the concurrency problem with, and depending on how that is solved, there may be a batter way of handling the seed.

On Thu, Dec 1, 2011 at 5:35 PM, Joe Williams reply@reply.github.com wrote:

Anders, you mentioned a patch for the random seed issue, I don't see that in these pull requests, is that something you wanted me to merge?


Reply to this email directly or view it on GitHub: https://github.com/boundary/folsom/pull/11#issuecomment-2983917

joewilliams commented 12 years ago

Ah cool, I see that now. Thanks!

joewilliams commented 12 years ago

merged