serial is updated but FetchFile returns 304 for objects and slurm is local and not changed #114
Labels
bug
Something isn't working
Comments
|
Are you sure nothing was updated? The |
|
I am seeing in the logs, so I know that I exit here which means that Upon further inspection, I see that there is a check for actual sha change in the maybe a similar block needs to be added on the slurm func ? (Thanks for rpki-client btw, I am using this wrapper: https://github.com/ties/rpki-client-web/blob/27a08aaec3244e5183b37ab511d3bd3d690c26f0/rpkiclientweb/web.py#L74) |
|
I tried a fix by using the lastUpdate time of the slurm file and doing a sha when on disk: https://github.com/netixx/stayrtr/blob/local-file-cache/utils/utils.go#L163 but now I get what I think is another issue: It seems that |
|
I meant to say that there always are ROAs about to expire: This expiration moment is communicated from rpki-client to StayRTR via the Have you excluded that this is what you are seeing? (I'm not excluding the possibility that there improvements to be made to stayrtr, but we have to make sure what exactly we're seeing) :-) |
|
BTW - rpki-client-web is a cool project, but there also might be benefit to having as little middleware in the pipeline as possible, for example:
|
|
Mmm... from your log: Old and New being the same values is a bit suspicious... if the |
|
I did some checks on an example file While the build time is "buildtime": "2024-02-27T17:04:42Z". So nothing should expire in the next 11 minutes after the build, however I still see serial update in the meantime: |
|
yup, that is fishy! Does |
|
I restarted stayrtr in with We can see that serial gets updated, but no actual diff are found. (I did some tests with rtrdump, but the example I used at that point did contain an update, so it was not highlighting the right behaviour) |
netixx
added a commit
to netixx/stayrtr
that referenced
this issue
Feb 28, 2024
Hash was already performed on cache/vrp data this performs it for slurm as well, preventing uneeded refreshes when neither file have changed. Partial fix for bgp#114.
netixx
added a commit
to netixx/stayrtr
that referenced
this issue
Feb 28, 2024
Hash was already performed on cache/vrp data this performs it for slurm as well, preventing uneeded refreshes when neither file have changed. Partial fix for bgp#114.
|
It looks like the However the diff between new and old is 0 ( There is a possibility that the count doesn't take into account duplicates (and I guess sdCurrent does not contain duplicates), however, I didn't see any duplicates in my vrp file from rpki-client |
|
I don't understand how you can get a 304 error for a local file? This can only be reached if This is what I see with: So it seems that the check if the update is different is not working. |
cjeker
added a commit
to cjeker/stayrtr
that referenced
this issue
Feb 29, 2024
…oes not include the vap and bgpsec keys. As a result reloadFromCurrentState() gets utterly confused and creates new versions when there is no change. Fix for bgp#114
|
The count only includes the roa payloads since it is countv4 + countv6. Fix for this is in #116 |
I was running a test patch that did a check for local files as well (I finally when another way, more in-line with the implementation used for the cache file). |
netixx
added a commit
to netixx/stayrtr
that referenced
this issue
Feb 29, 2024
Hash was already performed on cache/vrp data this performs it for slurm as well, preventing uneeded refreshes when neither file have changed. Partial fix for bgp#114.
|
I think the change I submitted is still valid, since I think there were actually 2 issues here: case. The fix you submitted covers the other branch of code: With my change: Without |
netixx
added a commit
to netixx/stayrtr
that referenced
this issue
Feb 29, 2024
Hash was already performed on cache/vrp data this performs it for slurm as well, preventing uneeded refreshes when neither file have changed. Partial fix for bgp#114.
netixx
pushed a commit
to netixx/stayrtr
that referenced
this issue
Feb 29, 2024
…oes not include the vap and bgpsec keys. As a result reloadFromCurrentState() gets utterly confused and creates new versions when there is no change. Fix for bgp#114
|
I believe all that's required is done. Thank you for all the work :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hello,
I added the serial metric in #113 and then I saw that it was continuously increasing.
In the logs, I see that the objects are not updated:
However, I also see that a new "update" is triggered.
I have a static
slurm.jsonfile provided as a local path in the container (via-slurm /slurm.jsonoption), and it's never updated.I think the serial should only be updated if the vrps are updated or the slurm is changed, and that seems to be what the code want's to do as well:
So I think there is unexpected behavior in :
Maybe the
FetchConfigcould maintain a hash of the file (when it's a local file) to see if it has been modified (emulate etag behavior for local files) ?Or maybe it should be in the
stateobject ?I you provide guidance I am happy to implement :)
The text was updated successfully, but these errors were encountered: