Date   
[EdgeX-TSC-Core] CBOR - Re-encode Performance Metrics

Anthony Bonafide <anthonymbonafide@...>
 

Hello all,

    Last week during the Core Working Group call we were discussing a couple of ways to address some issues regarding capturing ID and time information when dealing with CBOR content in core-data. One option was to decode the original content, update the necessary data and re-encode to CBOR before publishing. Before going down that path we wanted to grab some performance data on the re-encoding process. I have created a Github repo which simulates the process of re-encoding similar to how we would want to implement the previously mentioned steps. There are instructions in the README on how to run the benchmark tests, along with the test structure.

     In a nutshell, the benchmark accepts 2 arguments. The first specifies how many iterations to execute, the second specifies the Event payload size. The options for the payload size are small, medium, and large which correspond to 100K,900K,and 12M respectively(to match the results shared by Tobias regarding checksum implementation). The logic for the bench mark works as follows:
  1. Create CBOR bytes of an Event with a payload of the specified size
  2.  Capture system metrics and capture the start time
  3. Decode CBOR to an Event
  4. Change the ID and Pushed fields
  5. Encode the updated Event
  6. Repeat steps 3-5 as per the specified number of iterations
  7. Calculate elapsed time
  8. Display metrics

    A run of the tests on my laptop resulted in the following:

Metric Event with 100K payload Event with 900K payload Event with 12M payload
Average Re-encode time(nano seconds) 4148 4497 4554
GC runs 590 755 186
GC stop the world time(milliseconds) 57 71 21
Memory allocations(Memory at test completion – memory before test) 30000096 30000148 30000046
System memory in bytes (Memory test completion – memory before test) 1179648 1441792 1179648

    There is more information provided by the CLI tool, but those are what I felt might be the most important. Also, there are a few Go Benchmark tests in the repo which benchmark encoding, decoding, and the re-encoding process as listed above. Each of the tests are isolated and provide another way to get performance metrics. The instructions for executing the Go Benchmark tests can also be found in the Go Benchmark Section of the README. Hopefully this helps and if anyone has any ideas or suggestions regarding more and/or better metrics feel free to reach out.


Thank you,

Anthony Bonafide

Re: [EdgeX-TSC-Core] CBOR - Re-encode Performance Metrics

Tobias Mosby
 

Yes, this is very helpful to see the metrics we can anticipate for re-encode, garbage collection, and memory utilization.

Thank you Anthony!

 

Are the timings you measured for re-encode in units of microseconds?  I re-ran and was looking to decode the actual value; will follow up with you.

 

This morning we discussed sharing anticipated response times for SHA256 vs MD5 checksum hashing algorithms.

Eric also suggested a look at XXHash so it is included below for comparison as well.

 

NOTES:

·         CBOR encode/decode was performed using go-codec v1.1.4 which is not yet added to EdgeX codebase. I also found that setting up a buffered reader adversely impacts CBOR encode performance.

·         Each run was provided same three payload sizes (small/medium/large), performed across 1k iterations respectively, with rolling average used to produce result set.

·         Only intentional difference between each run was to apply a different checksum method to compute the event/payload hash.

·         Takeaway: As seen in Run #3, xxHash.Checksum64 is demonstrably more performant. So if “weaker” (but speedy) hashes such as this were used in place of uuid and re-encode of the event model, mitigation of collisions may need to be addressed at persistence layer. For example, FIFO to resolve which event record to mark “pushed” if/when distinct events are assigned the same hash value; ref: https://github.com/Cyan4973/xxHash/issues/165

 

Run #1

Dimension

100kb Payload

900kb Payload

12MB Payload

Binary Image Load:

53µs

439µs

3850µs

CBOR Encode:

33µs

177µs

1700µs

Checksum (SHA256):

598µs

6000µs

85000µs

CBOR Decode:

90µs

1000µs

81000µs

Overall Iteration Time:

0.75ms

7.5ms

171ms

TOTAL Time (1000 iterations)

0.8s

7.5s

2m51s

 

Run #2

Dimension

100kb Payload

900kb Payload

12MB Payload

Binary Image Load:

49µs

452µs

3900µs

CBOR Encode:

50µs

205µs

1900µs

Checksum (MD5):

208µs

1900µs

24000µs

CBOR Decode:

63µs

980µs

77000µs

Overall Iteration Time:

0.37ms

3.5ms

107ms

TOTAL Time (1000 iterations)

0.4s

3.5s

1m47s

 

Run #3

Dimension

100kb Payload

900kb Payload

12MB Payload

Binary Image Load:

46µs

411µs

4000µs

CBOR Encode:

36µs

182µs

1800µs

Checksum (xxHash):

15µs

121µs

1700µs

CBOR Decode:

64µs

805µs

84000µs

Overall Iteration Time:

0.16ms

1.5ms

91ms

TOTAL Time (1000 iterations)

162ms

1.5s

7m36s

 

*Resulting metrics in the tables above must be taken as anecdotal (mileage may vary) since these were executed on my laptop in a VM with other services such as EdgeX stack running.

 

Best regards,
Toby

 

From: EdgeX-TSC-Core@... [mailto:EdgeX-TSC-Core@...] On Behalf Of Anthony Bonafide
Sent: Monday, April 22, 2019 2:14 PM
To: EdgeX-GoLang@...; edgex-tsc-core@...
Subject: [EdgeX-TSC-Core] CBOR - Re-encode Performance Metrics

 

Hello all,

 

    Last week during the Core Working Group call we were discussing a couple of ways to address some issues regarding capturing ID and time information when dealing with CBOR content in core-data. One option was to decode the original content, update the necessary data and re-encode to CBOR before publishing. Before going down that path we wanted to grab some performance data on the re-encoding process. I have created a Github repo which simulates the process of re-encoding similar to how we would want to implement the previously mentioned steps. There are instructions in the README on how to run the benchmark tests, along with the test structure.

 

     In a nutshell, the benchmark accepts 2 arguments. The first specifies how many iterations to execute, the second specifies the Event payload size. The options for the payload size are small, medium, and large which correspond to 100K,900K,and 12M respectively(to match the results shared by Tobias regarding checksum implementation). The logic for the bench mark works as follows:

  1. Create CBOR bytes of an Event with a payload of the specified size
  2.  Capture system metrics and capture the start time
  3. Decode CBOR to an Event
  4. Change the ID and Pushed fields
  5. Encode the updated Event
  6. Repeat steps 3-5 as per the specified number of iterations
  7. Calculate elapsed time
  8. Display metrics

 

    A run of the tests on my laptop resulted in the following:

 

Metric

Event with 100K payload

Event with 900K payload

Event with 12M payload

Average Re-encode time(nano seconds)

4148

4497

4554

GC runs

590

755

186

GC stop the world time(milliseconds)

57

71

21

Memory allocations(Memory at test completion – memory before test)

30000096

30000148

30000046

System memory in bytes (Memory test completion – memory before test)

1179648

1441792

1179648

 

    There is more information provided by the CLI tool, but those are what I felt might be the most important. Also, there are a few Go Benchmark tests in the repo which benchmark encoding, decoding, and the re-encoding process as listed above. Each of the tests are isolated and provide another way to get performance metrics. The instructions for executing the Go Benchmark tests can also be found in the Go Benchmark Section of the README. Hopefully this helps and if anyone has any ideas or suggestions regarding more and/or better metrics feel free to reach out.

 

 

Thank you,

 

Anthony Bonafide

Re: [EdgeX-TSC-Core] CBOR - Re-encode Performance Metrics

Tobias Mosby
 

I had inadvertently recorded a timing for the last cell in the timings as 7m36s. This resulted from my run of 5000 iterations on the final check, in attempt to smooth out noise on average timings occurring between runs. Updated for consistency.

 

Note the variance in timings between runs will be reduced by running longer tests. We should not expect a chosen checksum method to impact encode/decode timings, etc.

 

From: Mosby, Tobias
Sent: Thursday, April 25, 2019 8:19 AM
To: 'Anthony Bonafide' <anthonymbonafide@...>; 'EdgeX-GoLang@...' <EdgeX-GoLang@...>; 'edgex-tsc-core@...' <edgex-tsc-core@...>; 'EdgeX-TSC-Device-Services@...' <EdgeX-TSC-Device-Services@...>
Subject: RE: [EdgeX-TSC-Core] CBOR - Re-encode Performance Metrics

 

Yes, this is very helpful to see the metrics we can anticipate for re-encode, garbage collection, and memory utilization.

Thank you Anthony!

 

Are the timings you measured for re-encode in units of microseconds?  I re-ran and was looking to decode the actual value; will follow up with you.

 

This morning we discussed sharing anticipated response times for SHA256 vs MD5 checksum hashing algorithms.

Eric also suggested a look at XXHash so it is included below for comparison as well.

 

NOTES:

·         CBOR encode/decode was performed using go-codec v1.1.4 which is not yet added to EdgeX codebase. I also found that setting up a buffered reader adversely impacts CBOR encode performance.

·         Each run was provided same three payload sizes (small/medium/large), performed across 1k iterations respectively, with rolling average used to produce result set.

·         Only intentional difference between each run was to apply a different checksum method to compute the event/payload hash.

·         Takeaway: As seen in Run #3, xxHash.Checksum64 is demonstrably more performant. So if “weaker” (but speedy) hashes such as this were used in place of uuid and re-encode of the event model, mitigation of collisions may need to be addressed at persistence layer. For example, FIFO to resolve which event record to mark “pushed” if/when distinct events are assigned the same hash value; ref: https://github.com/Cyan4973/xxHash/issues/165

 

Run #1

Dimension

100kb Payload

900kb Payload

12MB Payload

Binary Image Load:

53µs

439µs

3850µs

CBOR Encode:

33µs

177µs

1700µs

Checksum (SHA256):

598µs

6000µs

85000µs

CBOR Decode:

90µs

1000µs

81000µs

Overall Iteration Time:

0.75ms

7.5ms

171ms

TOTAL Time (1000 iterations)

0.8s

7.5s

2m51s

 

Run #2

Dimension

100kb Payload

900kb Payload

12MB Payload

Binary Image Load:

49µs

452µs

3900µs

CBOR Encode:

50µs

205µs

1900µs

Checksum (MD5):

208µs

1900µs

24000µs

CBOR Decode:

63µs

980µs

77000µs

Overall Iteration Time:

0.37ms

3.5ms

107ms

TOTAL Time (1000 iterations)

0.4s

3.5s

1m47s

 

Run #3

Dimension

100kb Payload

900kb Payload

12MB Payload

Binary Image Load:

46µs

411µs

4000µs

CBOR Encode:

36µs

182µs

1800µs

Checksum (xxHash):

15µs

121µs

1700µs

CBOR Decode:

64µs

805µs

84000µs

Overall Iteration Time:

0.16ms

1.5ms

91ms

TOTAL Time (1000 iterations)

162ms

1.5s

7m36s 1m33

 

*Resulting metrics in the tables above must be taken as anecdotal (mileage may vary) since these were executed on my laptop in a VM with other services such as EdgeX stack running.

 

Best regards,
Toby

 

From: EdgeX-TSC-Core@... [mailto:EdgeX-TSC-Core@...] On Behalf Of Anthony Bonafide
Sent: Monday, April 22, 2019 2:14 PM
To: EdgeX-GoLang@...; edgex-tsc-core@...
Subject: [EdgeX-TSC-Core] CBOR - Re-encode Performance Metrics

 

Hello all,

 

    Last week during the Core Working Group call we were discussing a couple of ways to address some issues regarding capturing ID and time information when dealing with CBOR content in core-data. One option was to decode the original content, update the necessary data and re-encode to CBOR before publishing. Before going down that path we wanted to grab some performance data on the re-encoding process. I have created a Github repo which simulates the process of re-encoding similar to how we would want to implement the previously mentioned steps. There are instructions in the README on how to run the benchmark tests, along with the test structure.

 

     In a nutshell, the benchmark accepts 2 arguments. The first specifies how many iterations to execute, the second specifies the Event payload size. The options for the payload size are small, medium, and large which correspond to 100K,900K,and 12M respectively(to match the results shared by Tobias regarding checksum implementation). The logic for the bench mark works as follows:

  1. Create CBOR bytes of an Event with a payload of the specified size
  2.  Capture system metrics and capture the start time
  3. Decode CBOR to an Event
  4. Change the ID and Pushed fields
  5. Encode the updated Event
  6. Repeat steps 3-5 as per the specified number of iterations
  7. Calculate elapsed time
  8. Display metrics

 

    A run of the tests on my laptop resulted in the following:

 

Metric

Event with 100K payload

Event with 900K payload

Event with 12M payload

Average Re-encode time(nano seconds)

4148

4497

4554

GC runs

590

755

186

GC stop the world time(milliseconds)

57

71

21

Memory allocations(Memory at test completion – memory before test)

30000096

30000148

30000046

System memory in bytes (Memory test completion – memory before test)

1179648

1441792

1179648

 

    There is more information provided by the CLI tool, but those are what I felt might be the most important. Also, there are a few Go Benchmark tests in the repo which benchmark encoding, decoding, and the re-encoding process as listed above. Each of the tests are isolated and provide another way to get performance metrics. The instructions for executing the Go Benchmark tests can also be found in the Go Benchmark Section of the README. Hopefully this helps and if anyone has any ideas or suggestions regarding more and/or better metrics feel free to reach out.

 

 

Thank you,

 

Anthony Bonafide

Community Feedback Required on PRs 1348 / 1359

Trevor.Conn@...
 

Hi all --


As you're aware from recent working group calls we need to develop a mechanism whereby the service configurations can be easily overridden to use Redis for the upcoming Edinburgh release. In a deployment scenario, we do not want to require an admin to go in and modify the configuration files for Redis to be enabled. It needs to be accomplished either via the Snap, docker-compose or script for automation.


We have two PRs for issue 1347 that seek to solve this problem. One of them is mine and I don't think it's appropriate for me to make a unilateral decision, so I'm seeking feedback from the community as to which approach should be accepted.


To summarize each approach:


1.) PR 1359 utilizes changes to the config-seed Dockerfile that creates copies of the existing service configuration.toml files. It replaces the necessary properties in the duplicated files with Redis values and then injects these into the config-seed image in a different directory. To use this, a Redis-specific docker-compose.yml would utilize the existing --cmd parameter to point at the root location of the Redis configuration files. For example: 

config-seed:
    image: edgexfoundry/docker-core-config-seed-go:0.7.1
    container_name: edgex-config-seed
    command: ["/edgex/cmd/config-seed/config-seed --profile=docker --cmd=/edgex/cmd-redis"]


2.) PR 1348 defines a new command line parameter on the config-seed (--database / -d) that defaults to Mongo. If the service is started with "-d=redis" then the database connectivity parameters are overridden from the existing configuration.toml files prior to population of Consul.


The essential differences are:

  • Definition of a new command line parameter on the config-seed versus not.
  • Assumptions about whether the config-seed should perform this function in native deployments utilizing Consul.
    • 1348 will accomplish this. 1359 will put this responsibility on the admin since it only touches the Dockerfile.
Since code freeze is on Tuesday the 28th, we need a consensus from the community rather quickly. You are welcome to provide feedback here or on either PR. If no clear preference has emerged by the time we hold the Core Working Group call this Thursday, we will make a decision then.


Thanks.


Trevor Conn
Technical Staff Engineer
Core Working Group Chair of EdgeX Foundry
Dell Technologies | IoT DellTech
Trevor_Conn@...
Round Rock, TX  USA

Re: Community Feedback Required on PRs 1348 / 1359

Goodell, Leonard
 

I prefer PR 1348 as it works for native and snaps, not just Docker. Also, I think it is more straight forward than modifying the toml file during the Docker build.

 

-Lenny

 

From: EdgeX-GoLang@... <EdgeX-GoLang@...> On Behalf Of Trevor.Conn@...
Sent: Tuesday, May 21, 2019 8:41 AM
To: EdgeX-GoLang@...; edgex-tsc-core@...; EdgeX-Devel@...
Subject: [Edgex-golang] Community Feedback Required on PRs 1348 / 1359

 

Hi all --

 

As you're aware from recent working group calls we need to develop a mechanism whereby the service configurations can be easily overridden to use Redis for the upcoming Edinburgh release. In a deployment scenario, we do not want to require an admin to go in and modify the configuration files for Redis to be enabled. It needs to be accomplished either via the Snap, docker-compose or script for automation.

 

We have two PRs for issue 1347 that seek to solve this problem. One of them is mine and I don't think it's appropriate for me to make a unilateral decision, so I'm seeking feedback from the community as to which approach should be accepted.

 

To summarize each approach:

 

1.) PR 1359 utilizes changes to the config-seed Dockerfile that creates copies of the existing service configuration.toml files. It replaces the necessary properties in the duplicated files with Redis values and then injects these into the config-seed image in a different directory. To use this, a Redis-specific docker-compose.yml would utilize the existing --cmd parameter to point at the root location of the Redis configuration files. For example: 

config-seed:
    image: edgexfoundry/docker-core-config-seed-go:0.7.1
    container_name: edgex-config-seed
    command: ["/edgex/cmd/config-seed/config-seed --profile=docker --cmd=/edgex/cmd-redis"]

 

2.) PR 1348 defines a new command line parameter on the config-seed (--database / -d) that defaults to Mongo. If the service is started with "-d=redis" then the database connectivity parameters are overridden from the existing configuration.toml files prior to population of Consul.

 

The essential differences are:

  • Definition of a new command line parameter on the config-seed versus not.
  • Assumptions about whether the config-seed should perform this function in native deployments utilizing Consul.
    • 1348 will accomplish this. 1359 will put this responsibility on the admin since it only touches the Dockerfile.

Since code freeze is on Tuesday the 28th, we need a consensus from the community rather quickly. You are welcome to provide feedback here or on either PR. If no clear preference has emerged by the time we hold the Core Working Group call this Thursday, we will make a decision then.

 

Thanks.

 

Trevor Conn
Technical Staff Engineer

Core Working Group Chair of EdgeX Foundry

Dell Technologies | IoT DellTech
Trevor_Conn@...
Round Rock, TX  USA

Re: Community Feedback Required on PRs 1348 / 1359

Ian Johnson
 

Apologies for the long email, but there's a bit of background necessary to explain why I don't think it's a good idea to use this command line flag in the snap.


With the snap we are in a better position than Docker to use the configuration files more directly because the configuration files live on the user's host file system and not inside the container where modification is tricky. Due to this, we try to strike a nice balance in the snap between allowing the user to modify the configuration files directly and also enable using consul for more automated device management tasks. Our current plan for managing this can be read about in issue #922.


Basically, in the snap we want to have 2 ways to run config-seed, one automated way which runs without the overwrite flag, and one manual way which runs with the overwrite flag. This allows a developer or manual user to easily change configuration for the services by modifying the files locally on their file system directly and then run a single command to push the content of the configuration from the filesystem into Consul.


I don’t think the addition of this new command line option will really help the snap at all. To explain that, let me explain how using redis currently would work. If a user wants to use redis our current mechanism would be just to have the user run:


snap set dbtype=redisdb


(or they could use the snapd REST API to do the same thing)


That then triggers a configuration hook to run, which processes the configuration.toml files locally for redis using a tool called remarshal, then pushes that configuration into consul so that an automated configuration management system can interact with consul to get/set configuration afterwards. This has the combined benefit of allowing a local user or developer to know what and where the configuration items that are set come from. They either originated from local configuration files or from something driving consul. The “snap set” mechanism described above merely is used as a transparent external means to modify something internal to EdgeX, the configuration files and Consul. It’s not meant to store state about EdgeX configuration, just to conveniently trigger those internal changes.


If we now consider using the command line flag that Trevor has proposed, if we are to use that option in the snap, we now need to track internally in the snap what database was configured with the “snap set” command above, and more importantly use that config item to determine what flags to launch config-seed with (dbtype=redis -> --database=redis, etc.). This means that in the snap, EdgeX now has an external system controlling configuration of the system and someone using the snap has a third dimension for control-plane/configuration:


  • Configuration files

  • Consul

  • Snapd configuration items


Previously, the snapd configuration items were only used to track what services were on/off, which is no different to someone using docker-compose to start/stop certain services and is a natural way to track this.


Our intention with the snap is to minimize the extent to which the snap distribution differs from the other distribution methods, but also to add convenient features which the snap can provide that aren’t available to docker-compose and native packaging methods. While it may be slightly simpler to add this command line flag for Redis to config-seed and use it within the snap, I think that it is simpler from a user’s perspective (as well as a documentation perspective) to have all the EdgeX configuration live within EdgeX. Having configuration items live in multiple locations significantly contributes to user confusion and complexity as the software ages and so trying to minimize this from the 1.0 release is beneficial.




On Tue, May 21, 2019 at 11:13 AM Goodell, Leonard <leonard.goodell@...> wrote:

I prefer PR 1348 as it works for native and snaps, not just Docker. Also, I think it is more straight forward than modifying the toml file during the Docker build.

 

-Lenny

 

From: EdgeX-GoLang@... <EdgeX-GoLang@...> On Behalf Of Trevor.Conn@...
Sent: Tuesday, May 21, 2019 8:41 AM
To: EdgeX-GoLang@...; edgex-tsc-core@...; EdgeX-Devel@...
Subject: [Edgex-golang] Community Feedback Required on PRs 1348 / 1359

 

Hi all --

 

As you're aware from recent working group calls we need to develop a mechanism whereby the service configurations can be easily overridden to use Redis for the upcoming Edinburgh release. In a deployment scenario, we do not want to require an admin to go in and modify the configuration files for Redis to be enabled. It needs to be accomplished either via the Snap, docker-compose or script for automation.

 

We have two PRs for issue 1347 that seek to solve this problem. One of them is mine and I don't think it's appropriate for me to make a unilateral decision, so I'm seeking feedback from the community as to which approach should be accepted.

 

To summarize each approach:

 

1.) PR 1359 utilizes changes to the config-seed Dockerfile that creates copies of the existing service configuration.toml files. It replaces the necessary properties in the duplicated files with Redis values and then injects these into the config-seed image in a different directory. To use this, a Redis-specific docker-compose.yml would utilize the existing --cmd parameter to point at the root location of the Redis configuration files. For example: 

config-seed:
    image: edgexfoundry/docker-core-config-seed-go:0.7.1
    container_name: edgex-config-seed
    command: ["/edgex/cmd/config-seed/config-seed --profile=docker --cmd=/edgex/cmd-redis"]

 

2.) PR 1348 defines a new command line parameter on the config-seed (--database / -d) that defaults to Mongo. If the service is started with "-d=redis" then the database connectivity parameters are overridden from the existing configuration.toml files prior to population of Consul.

 

The essential differences are:

  • Definition of a new command line parameter on the config-seed versus not.
  • Assumptions about whether the config-seed should perform this function in native deployments utilizing Consul.
    • 1348 will accomplish this. 1359 will put this responsibility on the admin since it only touches the Dockerfile.

Since code freeze is on Tuesday the 28th, we need a consensus from the community rather quickly. You are welcome to provide feedback here or on either PR. If no clear preference has emerged by the time we hold the Core Working Group call this Thursday, we will make a decision then.

 

Thanks.

 

Trevor Conn
Technical Staff Engineer

Core Working Group Chair of EdgeX Foundry

Dell Technologies | IoT DellTech
Trevor_Conn@...
Round Rock, TX  USA

Re: Community Feedback Required on PRs 1348 / 1359

Goodell, Leonard
 

Ian makes a good point in that in the non-docker scenarios, the configuration.toml is easily modified to specify use of Redis. So in those cases having a new command line switch is redundant.

 

I still think for the Docker scenario, the command line switch is preferred. I’d like the DB settings override be in code rather than in the Docker file. Have the switch doesn’t make it required in the non-docker scenarios. There the configuration.toml can still be modified, rather than use the switch.

 

-Lenny

 

From: EdgeX-GoLang@... <EdgeX-GoLang@...> On Behalf Of Ian Johnson
Sent: Tuesday, May 21, 2019 11:47 AM
To: Goodell, Leonard <leonard.goodell@...>
Cc: Trevor.Conn@...; EdgeX-GoLang@...; edgex-tsc-core@...; EdgeX-Devel@...
Subject: Re: [Edgex-golang] Community Feedback Required on PRs 1348 / 1359

 

Apologies for the long email, but there's a bit of background necessary to explain why I don't think it's a good idea to use this command line flag in the snap.

 

With the snap we are in a better position than Docker to use the configuration files more directly because the configuration files live on the user's host file system and not inside the container where modification is tricky. Due to this, we try to strike a nice balance in the snap between allowing the user to modify the configuration files directly and also enable using consul for more automated device management tasks. Our current plan for managing this can be read about in issue #922.

 

Basically, in the snap we want to have 2 ways to run config-seed, one automated way which runs without the overwrite flag, and one manual way which runs with the overwrite flag. This allows a developer or manual user to easily change configuration for the services by modifying the files locally on their file system directly and then run a single command to push the content of the configuration from the filesystem into Consul.

 

I don’t think the addition of this new command line option will really help the snap at all. To explain that, let me explain how using redis currently would work. If a user wants to use redis our current mechanism would be just to have the user run:

 

snap set dbtype=redisdb

 

(or they could use the snapd REST API to do the same thing)

 

That then triggers a configuration hook to run, which processes the configuration.toml files locally for redis using a tool called remarshal, then pushes that configuration into consul so that an automated configuration management system can interact with consul to get/set configuration afterwards. This has the combined benefit of allowing a local user or developer to know what and where the configuration items that are set come from. They either originated from local configuration files or from something driving consul. The “snap set” mechanism described above merely is used as a transparent external means to modify something internal to EdgeX, the configuration files and Consul. It’s not meant to store state about EdgeX configuration, just to conveniently trigger those internal changes.

 

If we now consider using the command line flag that Trevor has proposed, if we are to use that option in the snap, we now need to track internally in the snap what database was configured with the “snap set” command above, and more importantly use that config item to determine what flags to launch config-seed with (dbtype=redis -> --database=redis, etc.). This means that in the snap, EdgeX now has an external system controlling configuration of the system and someone using the snap has a third dimension for control-plane/configuration:

 

  • Configuration files
  • Consul
  • Snapd configuration items

 

Previously, the snapd configuration items were only used to track what services were on/off, which is no different to someone using docker-compose to start/stop certain services and is a natural way to track this.

 

Our intention with the snap is to minimize the extent to which the snap distribution differs from the other distribution methods, but also to add convenient features which the snap can provide that aren’t available to docker-compose and native packaging methods. While it may be slightly simpler to add this command line flag for Redis to config-seed and use it within the snap, I think that it is simpler from a user’s perspective (as well as a documentation perspective) to have all the EdgeX configuration live within EdgeX. Having configuration items live in multiple locations significantly contributes to user confusion and complexity as the software ages and so trying to minimize this from the 1.0 release is beneficial.

 

 

 

On Tue, May 21, 2019 at 11:13 AM Goodell, Leonard <leonard.goodell@...> wrote:

I prefer PR 1348 as it works for native and snaps, not just Docker. Also, I think it is more straight forward than modifying the toml file during the Docker build.

 

-Lenny

 

From: EdgeX-GoLang@... <EdgeX-GoLang@...> On Behalf Of Trevor.Conn@...
Sent: Tuesday, May 21, 2019 8:41 AM
To: EdgeX-GoLang@...; edgex-tsc-core@...; EdgeX-Devel@...
Subject: [Edgex-golang] Community Feedback Required on PRs 1348 / 1359

 

Hi all --

 

As you're aware from recent working group calls we need to develop a mechanism whereby the service configurations can be easily overridden to use Redis for the upcoming Edinburgh release. In a deployment scenario, we do not want to require an admin to go in and modify the configuration files for Redis to be enabled. It needs to be accomplished either via the Snap, docker-compose or script for automation.

 

We have two PRs for issue 1347 that seek to solve this problem. One of them is mine and I don't think it's appropriate for me to make a unilateral decision, so I'm seeking feedback from the community as to which approach should be accepted.

 

To summarize each approach:

 

1.) PR 1359 utilizes changes to the config-seed Dockerfile that creates copies of the existing service configuration.toml files. It replaces the necessary properties in the duplicated files with Redis values and then injects these into the config-seed image in a different directory. To use this, a Redis-specific docker-compose.yml would utilize the existing --cmd parameter to point at the root location of the Redis configuration files. For example: 

config-seed:
    image: edgexfoundry/docker-core-config-seed-go:0.7.1
    container_name: edgex-config-seed
    command: ["/edgex/cmd/config-seed/config-seed --profile=docker --cmd=/edgex/cmd-redis"]

 

2.) PR 1348 defines a new command line parameter on the config-seed (--database / -d) that defaults to Mongo. If the service is started with "-d=redis" then the database connectivity parameters are overridden from the existing configuration.toml files prior to population of Consul.

 

The essential differences are:

  • Definition of a new command line parameter on the config-seed versus not.
  • Assumptions about whether the config-seed should perform this function in native deployments utilizing Consul.
    • 1348 will accomplish this. 1359 will put this responsibility on the admin since it only touches the Dockerfile.

Since code freeze is on Tuesday the 28th, we need a consensus from the community rather quickly. You are welcome to provide feedback here or on either PR. If no clear preference has emerged by the time we hold the Core Working Group call this Thursday, we will make a decision then.

 

Thanks.

 

Trevor Conn
Technical Staff Engineer

Core Working Group Chair of EdgeX Foundry

Dell Technologies | IoT DellTech
Trevor_Conn@...
Round Rock, TX  USA