HPCC-31689 Add checkfilesize to daliadmin #18582
Conversation
|
Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-31689 Jirabot Action Result: |
| printf(" dfspart <logicalname> <part> -- get meta information for part num\n"); | ||
| printf(" checksuperfile <superfilename> [fix=true|false] -- check superfile links consistent and optionally fix\n"); | ||
| printf(" checksubfile <subfilename> -- check subfile links to parent consistent\n"); | ||
| printf(" checkfilesize <logicalfilemask> [fix=true|false] -- check file size attributes and optionally fix"); |
There was a problem hiding this comment.
NB: this is the new command, the other changes here are only alphabetic reordering
jakesmith
force-pushed
the
HPCC-31689-fixfilesize
branch
from
6e0f471 to
2999667
Compare
| @@ -148,6 +149,8 @@ int main(int argc, const char* argv[]) | |||
| StringBuffer tmps; | |||
| for (int i=1;i<argc;i++) { | |||
| const char *param = argv[i]; | |||
| if (startsWith(param, "--")) | |||
There was a problem hiding this comment.
NB: this is added so --config=/etc/config/.. can be added to the command line, which will be needed on a AKS system, to pickup plane configuration (from global) for path striping.
There was a problem hiding this comment.
Do we need to add param to params here ?
params.append(param);
There was a problem hiding this comment.
ok - I see now loadConfiguration() uses argv so we just skip param here.
dali/daliadmin/daadmin.cpp
Outdated
| offset_t partSize; | ||
| try | ||
| { | ||
| partSize = part.getFileSize(true, true); |
There was a problem hiding this comment.
Dont really need to call getFileSize() if fix == false ?
There was a problem hiding this comment.
you're right, I think I was originally thinking of logging it even when fix=false, but.. I'll move it in the if (fix) block.
|
@jakesmith looks good - squash-worthy |
jakesmith
force-pushed
the
HPCC-31689-fixfilesize
branch
from
c756d2b to
19a25f9
Compare
|
@mckellyln - squashed. |
|
Good to merge |
dali/daliadmin/daadmin.cpp
Outdated
| } | ||
| if (!locked) | ||
| { | ||
| part.lockProperties(30000); |
There was a problem hiding this comment.
Should this be protected by the catch as well, in case there is a weird timeout?? I think very unlikely.
...
Fine as it stands - the outer catch is sufficient.
There was a problem hiding this comment.
would be better in the catch I agree. I'll move.
dali/daliadmin/daadmin.cpp
Outdated
| if (!locked) | ||
| { | ||
| part.lockProperties(30000); | ||
| locked = true; |
There was a problem hiding this comment.
This is using knowledge of the internal implementation to avoid locking multiple times. Should it lock the file instead to make it explicit??
also, there should be a corresponding unlockProperties.
There was a problem hiding this comment.
I agree, locking just the file isn't really right either - that would also be using knowledge of internal implementation (i.e. that locking file equates to part attributes being protected too).
I will modify.
also, there should be a corresponding unlockProperties.
there is, via COnScopeExit ensureUnlock.
There was a problem hiding this comment.
I agree, locking just the file isn't really right either - that would also be using knowledge of internal implementation (i.e. that locking file equates to part attributes being protected too).
I will modify.
it was also wrong (but harmless) in the sense that the underlying implementation relies on knowing if each part is 'dirty', so should lock each part as it goes (I've changed it). It's a NOP though, if the file is already locked (which I've changed it to be).
| break; | ||
| else if (timer.elapsedCycles() >= queryOneSecCycles()*10) // log every 10 secs | ||
| { | ||
| PROGLOG("Processed %u files", count); |
There was a problem hiding this comment.
Could this tracing become excessive (e.g. if run as an esp service)?
There was a problem hiding this comment.
this is never intended to be run as a service - it's very specific to the missing @SiZe issue..
I think if it ever becomes something else, excessive tracing/other issues can be looked at then.
Add a command ('checkFileSize'), that given a logical filename
or a mask, scans logical files for parts with missing @SiZe
attributes, reports them, and optionally fixes them.
See HPCC-31662 for background.
Signed-off-by: Jake Smith <[email protected]>
jakesmith
force-pushed
the
HPCC-31689-fixfilesize
branch
from
6e19bb4 to
96bbb26
Compare
|
@ghalliday - squashed. |
Add a command ('checkFileSize'), that given a logical filename or a mask, scans logical files for parts with missing @SiZe attributes, reports them, and optionally fixes them. See HPCC-31662 for background.
Type of change:
Checklist:
Smoketest:
Testing: