You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by GitBox <gi...@apache.org> on 2020/09/23 02:24:13 UTC
[GitHub] [celix] unitink72 opened a new pull request #285: Add API call for retrieving db index
unitink72 opened a new pull request #285:
URL: https://github.com/apache/celix/pull/285
Its hard to use the ETCD watch API without knowing the current DB index, so I added it. It works when I test it. Hopefully the method of doing a GET on host/v2/keys just to get the HTTP header is acceptable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo edited a comment on pull request #285: Add API call for retrieving db index
Posted by GitBox <gi...@apache.org>.
Oipo edited a comment on pull request #285:
URL: https://github.com/apache/celix/pull/285#issuecomment-698164298
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo commented on a change in pull request #285: Add API call for retrieving db index
Posted by GitBox <gi...@apache.org>.
Oipo commented on a change in pull request #285:
URL: https://github.com/apache/celix/pull/285#discussion_r496224385
##########
File path: libs/etcdlib/api/etcdlib.h
##########
@@ -149,6 +149,13 @@ int etcdlib_del(etcdlib_t *etcdlib, const char* key);
*/
int etcdlib_watch(etcdlib_t *etcdlib, const char* key, long long index, char** action, char** prevValue, char** value, char** rkey, long long* modifiedIndex);
+/**
+ * @desc Retrieve the current database index
+ * @param const etcdlib_t* etcdlib. The ETCD-LIB instance
+ * @param int* modifiedIndex. The X-Etcd-Index value as retrieved from the etcd server.
+ */
+int etcdlib_get_db_index(etcdlib_t *etcdlib, int* modifiedIndex);
Review comment:
Please rename modifiedIndex to currentEtcdIndex
##########
File path: libs/etcdlib/src/etcd.c
##########
@@ -257,6 +257,40 @@ static long long etcd_get_current_index(const char* headerData) {
}
+int etcdlib_get_db_index(etcdlib_t *etcdlib, int* modifiedIndex) {
Review comment:
Please rename modifiedIndex to currentEtcdIndex
##########
File path: libs/etcdlib/src/etcd.c
##########
@@ -257,6 +257,40 @@ static long long etcd_get_current_index(const char* headerData) {
}
+int etcdlib_get_db_index(etcdlib_t *etcdlib, int* modifiedIndex) {
+
+ int res = -1;
+ struct MemoryStruct reply;
+
+ reply.memory = malloc(1); /* will be grown as needed by the realloc above */
+ reply.memorySize = 0; /* no data at this point */
+ reply.header = malloc(1); /* will be grown as needed by the realloc above */
+ reply.headerSize = 0; /* no data at this point */
+
+ int retVal = ETCDLIB_RC_OK;
+ char *url;
+ asprintf(&url, "http://%s:%d/v2/keys", etcdlib->host, etcdlib->port);
+ res = performRequest(&etcdlib->curl, &etcdlib->mutex, url, GET, NULL, (void *) &reply);
+ free(url);
+
+ if (res == CURLE_OK) {
+ long long indexFromHeader = etcd_get_current_index(reply.header);
+ *modifiedIndex = (int)indexFromHeader;
+ } else if (res == CURLE_OPERATION_TIMEDOUT) {
+ retVal = ETCDLIB_RC_TIMEOUT;
+ } else {
+ retVal = ETCDLIB_RC_ERROR;
+ fprintf(stderr, "Error getting etcd value, curl error: '%s'\n", curl_easy_strerror(res));
+ }
+
+ if (reply.memory) {
Review comment:
Although it happens multiple times in the current code, would you mind removing the if statement here? `free()` explicitly handles NULL values properly by ignoring them.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] unitink72 commented on pull request #285: Add API call for retrieving db index
Posted by GitBox <gi...@apache.org>.
unitink72 commented on pull request #285:
URL: https://github.com/apache/celix/pull/285#issuecomment-698421443
I had two factors that led me to wanting this available:
1. When I wrote my db interface code, the first thing I did was setup a directory/key structure then set a watch on the etcd directory containing all those keys. Using the index from the last get always resulted in the watch api being triggered on changes that were made in the past. There was an ugly work around that by writing a key that was outside that dir structure then reading it again, but that was a hack.
2. In the link you gave (etcd v2 api), there's a section down further titled "Watch From Cleared Event Index". It describes a case that could happen in my system - the modified index of the key being over 1000 revisions old. This would break my watch api calls.
It may be possible to have multiple X-Etcd-Index values among a cluster, but I'm assuming that they try to sycn them up asap. That issue doesn't bother me for my system.
If the code doesnt fit for your project, thats OK. Just thought I'd offer it up as its definitely useful for me.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo edited a comment on pull request #285: Add API call for retrieving db index
Posted by GitBox <gi...@apache.org>.
Oipo edited a comment on pull request #285:
URL: https://github.com/apache/celix/pull/285#issuecomment-698164298
So X-Etcd-Index is tied to an etcd server (so in a cluster, you can have multiple I think, not sure).
But the index used in the api is the modifiedIndex stored with a key.
If we take a look at the [etcd API](https://etcd.io/docs/v2/api/) description:
> node.createdIndex: an index is a unique, monotonically-incrementing integer created for each change to etcd. This specific index reflects the point in the etcd state member at which a given key was created. You may notice that in this example the index is 2 even though it is the first request you sent to the server. This is because there are internal commands that also change the state behind the scenes, like adding and syncing servers.
> node.modifiedIndex: like node.createdIndex, this attribute is also an etcd index. Actions that cause the value to change include set, delete, update, create, compareAndSwap and compareAndDelete. Since the get and watch commands do not change state in the store, they do not change the value of node.modifiedIndex.
It's probably best to do an `etcdlib_get()` before an `etcdlib_watch()`. If the return value is ETCDLIB_RC_ERROR, you can assume it is 0.
Verifying this locally:
```
oipo@oipo-X570-AORUS-ELITE:~$ curl http://127.0.0.1:2379/v2/keys/message3
{"errorCode":100,"message":"Key not found","cause":"/message3","index":5266}
oipo@oipo-X570-AORUS-ELITE:~$ curl http://127.0.0.1:2379/v2/keys
{"action":"get","node":{"dir":true,"nodes":[{"key":"/hier","dir":true,"modifiedIndex":13,"createdIndex":13}]}}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] unitink72 commented on pull request #285: Add API call for retrieving db index
Posted by GitBox <gi...@apache.org>.
unitink72 commented on pull request #285:
URL: https://github.com/apache/celix/pull/285#issuecomment-698421443
I had two factors that led me to wanting this available:
1. When I wrote my db interface code, the first thing I did was setup a directory/key structure then set a watch on the etcd directory containing all those keys. Using the index from the last get always resulted in the watch api being triggered on changes that were made in the past. There was an ugly work around that by writing a key that was outside that dir structure then reading it again, but that was a hack.
2. In the link you gave (etcd v2 api), there's a section down further titled "Watch From Cleared Event Index". It describes a case that could happen in my system - the modified index of the key being over 1000 revisions old. This would break my watch api calls.
It may be possible to have multiple X-Etcd-Index values among a cluster, but I'm assuming that they try to sycn them up asap. That issue doesn't bother me for my system.
If the code doesnt fit for your project, thats OK. Just thought I'd offer it up as its definitely useful for me.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo edited a comment on pull request #285: Add API call for retrieving db index
Posted by GitBox <gi...@apache.org>.
Oipo edited a comment on pull request #285:
URL: https://github.com/apache/celix/pull/285#issuecomment-698164298
I believe the X-Etcd-Index value is tied to a key.
If we take a look at the [etcd API](https://etcd.io/docs/v2/api/) description:
> node.createdIndex: an index is a unique, monotonically-incrementing integer created for each change to etcd. This specific index reflects the point in the etcd state member at which a given key was created. You may notice that in this example the index is 2 even though it is the first request you sent to the server. This is because there are internal commands that also change the state behind the scenes, like adding and syncing servers.
It's probably best to do an `etcdlib_get()` before an `etcdlib_watch()`. If the return value is ETCDLIB_RC_ERROR, you can assume it is 0.
Verifying this locally:
```
oipo@oipo-X570-AORUS-ELITE:~$ curl http://127.0.0.1:2379/v2/keys/message3
{"errorCode":100,"message":"Key not found","cause":"/message3","index":5266}
oipo@oipo-X570-AORUS-ELITE:~$ curl http://127.0.0.1:2379/v2/keys
{"action":"get","node":{"dir":true,"nodes":[{"key":"/hier","dir":true,"modifiedIndex":13,"createdIndex":13}]}}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo commented on pull request #285: Add API call for retrieving db index
Posted by GitBox <gi...@apache.org>.
Oipo commented on pull request #285:
URL: https://github.com/apache/celix/pull/285#issuecomment-698164298
I believe the X-Etcd-Index value is tied to a key.
If we take a look at the [etcd API](https://etcd.io/docs/v2/api/) description:
> node.createdIndex: an index is a unique, monotonically-incrementing integer created for each change to etcd. This specific index reflects the point in the etcd state member at which a given key was created. You may notice that in this example the index is 2 even though it is the first request you sent to the server. This is because there are internal commands that also change the state behind the scenes, like adding and syncing servers.
It's probably best to do an `etcdlib_get()` before an `etcdlib_watch()`. If the return value is ETCDLIB_RC_ERROR, you can assume it is 0.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo commented on pull request #285: Add API call for retrieving db index
Posted by GitBox <gi...@apache.org>.
Oipo commented on pull request #285:
URL: https://github.com/apache/celix/pull/285#issuecomment-698164298
I believe the X-Etcd-Index value is tied to a key.
If we take a look at the [etcd API](https://etcd.io/docs/v2/api/) description:
> node.createdIndex: an index is a unique, monotonically-incrementing integer created for each change to etcd. This specific index reflects the point in the etcd state member at which a given key was created. You may notice that in this example the index is 2 even though it is the first request you sent to the server. This is because there are internal commands that also change the state behind the scenes, like adding and syncing servers.
It's probably best to do an `etcdlib_get()` before an `etcdlib_watch()`. If the return value is ETCDLIB_RC_ERROR, you can assume it is 0.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo edited a comment on pull request #285: Add API call for retrieving db index
Posted by GitBox <gi...@apache.org>.
Oipo edited a comment on pull request #285:
URL: https://github.com/apache/celix/pull/285#issuecomment-698164298
So X-Etcd-Index is tied to an etcd server (so in a cluster, you can have multiple I think, not sure).
But the index used in the api is the modifiedIndex stored with a key.
If we take a look at the [etcd API](https://etcd.io/docs/v2/api/) description:
> node.createdIndex: an index is a unique, monotonically-incrementing integer created for each change to etcd. This specific index reflects the point in the etcd state member at which a given key was created. You may notice that in this example the index is 2 even though it is the first request you sent to the server. This is because there are internal commands that also change the state behind the scenes, like adding and syncing servers.
> node.modifiedIndex: like node.createdIndex, this attribute is also an etcd index. Actions that cause the value to change include set, delete, update, create, compareAndSwap and compareAndDelete. Since the get and watch commands do not change state in the store, they do not change the value of node.modifiedIndex.
It's probably best to do an `etcdlib_get()` before an `etcdlib_watch()`. If the return value is ETCDLIB_RC_ERROR, you can assume it is 0.
Verifying this locally:
```
oipo@oipo-X570-AORUS-ELITE:~$ curl http://127.0.0.1:2379/v2/keys/message3
{"errorCode":100,"message":"Key not found","cause":"/message3","index":5266}
oipo@oipo-X570-AORUS-ELITE:~$ curl http://127.0.0.1:2379/v2/keys
{"action":"get","node":{"dir":true,"nodes":[{"key":"/hier","dir":true,"modifiedIndex":13,"createdIndex":13}]}}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org