PP-464: PBS Accounting Logs - Incorrect Escaping, Proposed Design posted


#1

Hi All,

I had posted my analysis and Proposed Design for PP-464 yesterday.

Request inputs/comments/signoff.

Thanks,
Prakash


#2

These accounting_logs changes may caused problems with a feature that will be checked-in soon to OSS. It’s PP-388 and here’s the EDD:

https://pbspro.atlassian.net/wiki/display/PD/PP-388%3A+Allow+mom+hooks+to+accumulate+resources_used+values


In the EDD, the string value of accumulated string resources will be in JSON-format, which will have the quotes in them. Please take some of the examples and show how it would look under PP-464. For example,

resources_used.foo_str=’{“a”: 1, “b”: 2, “c”:1,“d”: 4}’


#3

Also in the EDD, I see the following:

While reading the values, each escaped quote will become part of the value.
While reading the values, each occurrence of “\”, will be replaced with a ‘’.
While reading the values, the enclosing quotes (if any) will be dropped.

-> Who is reading the accounting_logs value? It can be many different utilities not under the control of PBS. Is this something that we document and tell accounting logs utility writer to follow?


#4

Internally, tracejob tool reads the accounting files.
I believe, PBSA too might be reading them.
Yes, this is something that we would need to document for any possible external accounting logs utility writers to follow.


#5

Regarding the conflict with that this change might have with the feature that you are working on, I am of the belief that these changes will be using the “values” and insert escape characters while storing them in the accounting logs. I do not think the format of the value will have any effect on these changes.
OR Am I missing anything obvious?

Thanks,
Prakash


#6

Please change the title of this posting so that it matches all the existing design discussions. Design discussions should have a title that exactly matches the JIRA ticket, e.g.: PP-###:

.

Please remove the “EDD for” at the beginning of this one to be consistent.

Thanks!


#7

@billnitzberg, made the change, is it ok now?

Thanks,
Prakash


#8

@smgoosen’s questions
Hi Prakash,

I was thinking each step would have an example.

Questions I still have:
If there are no non-alphanumeric cahracters in the value then there are no quotes around the value in the accounting log?
When you say “While reading…” what do you mean? You specifically mean tracejob, right? Customer’s tools might have to be modified?

Thanks,
Sam Goosen


#9

Could you please clarify, what you mean by each step.

correct. we need the quotes only if we have non alphanumeric characters in the value.

any tool/utility that reads the accounting logs file, would need to be
modified. This is what I had said when the discussions were happening
earlier, but the field wanted this solution to be implemented.


#10

@smgoosen, I have updated the Proposed Design with examples for every statement. Could you please provide your comments/sign-off.

Thanks,
Prakash


#11

The EDD looks good, thanks for adding the examples


#12

How will this work with PP-409? It’s EDD can be found at https://pbspro.atlassian.net/wiki/display/PD/PP-388%3A+Allow+mom+hooks+to+accumulate+resources_used+values.


#13

Jon,

Until I am missing anything, PP-464 works on the values and how they are written and read from the accounting logs, so PP-409 and PP-464 should not interfere.

Thanks,
Prakash


#14

@prakashcv13: Will PP-464 also modify the value recorded in resources_used? Then that could conflict with PP-409, which will have JSON values like:

esources_used.foo_str=’{“nine”: 9, “ten”: 10}’ resources_used.foo_assn2=’{“vn1”: 1, “vn2”: 2 ,“vn3”: 3 ,“vn4”: 4, “vn5”: 5, “vn6”: 6}’


#15

@bayucan, PP-464 does not modify the value stored in resources_used.{resource_name}. It does change how these values get recorded in the accounting logs.


#16

I went ahead and tested the PP-409 code with the code in PP-464 and I’m definitely seeing a difference in accounting_logs output…

For example, before:
resources_used.foo_str="’{“eight”: 8, “seven”: 7, “nine”: 9}’“
with PP-464, it’s now:
resources_used.foo_str=”’{“eight”: 8, “seven”: 7, “nine”: 9}’"


#17

yes, but the value within PBS doesn’t change. It is only how these values get stored in the accounting logs.
I am not sure, if we are on the same page about how we perceive “conflict”. Could you please make me understand.


#18

This is a problem. The point of making the output json was so that admins could easily read it in with a json reader. I took the output and tried to read it back in and it failed.

a = "’{“eight”: 8, “seven”: 7, “nine”: 9}’"
a
’’{“eight”: 8, “seven”: 7, “nine”: 9}’’

import json
b = json.loads(a)
Traceback (most recent call last):
File “”, line 1, in
File “/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/json/init.py”, line 338, in loads
return _default_decoder.decode(s)
File “/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/json/decoder.py”, line 366, in decode
obj, end = self.raw_decode(s, idx=_w(s, 0).end())
File “/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/json/decoder.py”, line 384, in raw_decode
raise ValueError(“No JSON object could be decoded”)
ValueError: No JSON object could be decoded

When I brought this up months ago to Prakash and Sam on the forum I thought I was assured that it would not be a problem. Recommendations on a fix? Do we need to back out PBS-13646?


#19

The issue is that a json formatted string should be able to read directly in. However, this fix breaks it. Now this would work if we removed the single quotes “\’{}\’”. But with the leading and the trailing double quotes it makes the string unreadable to the json module. We don’t want to have make any python/perl/etc script to have to do something special to read in a json string. That would defeat the purpose. One fix would be to have your code replace the single quotes if you are going to put leading double quotes. The other option is to not add leading and trailing double quotes if it starts and ends with single quotes.


#20

@jon, @smgoosen, I have updated the EDD, request you to go through the same and let me know if any changes are needed.