Skip to content

KxDashboards - Debugging .kxdash.exec #651

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
davidbuckley123 wants to merge 4 commits into
base: master
Choose a base branch
from
Open

KxDashboards - Debugging .kxdash.exec #651

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4f6d141
Update kxdash.q
davidbuckley123 Apr 29, 2024
f456e58
Update gateway.q
davidbuckley123 Apr 29, 2024
eb49345
Update kxdash.q
davidbuckley123 Apr 29, 2024
d159c25
Update .kxdash.dashps from kxdash.q
davidbuckley123 Apr 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 6 additions & 11 deletions code/gateway/kxdash.q
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ dashparams:`o`w`r`limit!(0;0i;0i;0W)

// function to be called from the dashboards
dashexec:{[q;s;j]
.gw.asyncexecjpt[(dashremote;q;dashparams);(),s;dashjoin[j];();0Wn]
.gw.syncexecj[q;(),s;j]
}

// execute the request
Expand All @@ -24,18 +24,13 @@ dashjoin:{[joinfunc;r]
(`.dash.snd_err;r[0;1;`w];r[0;1;`r];r[0;1;`result])]
}

// this function is to be inserted into .z.ps to set the .kxdash.dashparams variable
dashps:{
// check the query coming in meets the format
$[@[{`f`w`r`x`u~first 1_ value first x};x;0b];
if[@[{`f`w`r`x`u~first 1_ value first x};x;0b];
// pull out the values we need to return to the dashboards
[dashparams::`o`w`r`limit!(last value x 1;x 2;x 3;x[4;0]);
// execute the query part, which must look something like
// .kxdash.dashexec["select from t";`rdb`hdb;raze]
value x[4;1];
];
//
value x]
}
dashparams::`o`w`r`limit!(last value x 1;x 2;x 3;x[4;0])];
};


// need this to handle queries that only hit one backend process
Expand All @@ -57,5 +52,5 @@ init:{
// incorporate dashps into the .z.ps definition
.dotz.set[`.z.ps;{x@y;.kxdash.dashps y}@[value;.dotz.getcommand[`.z.ps];{{value x}}]];
Copy link
Member

@jonathonmcmurray jonathonmcmurray May 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.kxdash.dashps is executed after the original .z.ps definition - are we actually using dashparams anywhere after this change? or the dashremote and dashjoin functions? Seems like most of this script is possibly redundant if we're just using the original .z.ps?

};

if[enabled;init[]];
2 changes: 1 addition & 1 deletion code/processes/gateway.q
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ asyncexec:asyncexecjpt[;;raze;();0Wn]
// execute a synchronous query
syncexecjpre36:{[query;servertype;joinfunction]
// Check correct function called
if[(.z.w<>0)&(.z.w in key .gw.call)&not .gw.call .z.w;
if[(.z.w<>0)&(.z.w in key .gw.call)&(not .gw.call .z.w)& not @[value;`.kxdash.enabled;0b];
Copy link
Member

@jonathonmcmurray jonathonmcmurray May 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will allow syncexec to be called via async request as long as .kxdash.enabled is true i.e. it'd allow a non-kx dashboard client to send async request to syncexec, which I don't think is what we want?

it also seems strange to me that we are using syncexecjpre36 for async requests from kx dashboards, is this definitely the correct approach? (I guess this is because kx dashboards is essentially doing a defferred sync request with it's own wrapper, so it's simpler on our side to treat it as a sync request, so this probably does make sense)

Copy link
Contributor

@jonnypress jonnypress Jun 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidbuckley123 couple of points to pick up here if you get a chance

Copy link
Author

@davidbuckley123 davidbuckley123 Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-kx dashboard client to send async request to syncexec
Fair enough - the if[] statement could be tightened up a bit.

I am only trying to enabled syncexec calls from a negative handle from kx dashboards.
Honestly, I am not 100% satisfied with this as a solution.
I'll take some time and see if I can come up with something more elegant.

@[neg .z.w;.gw.formatresponse[0b;0b;"Incorrect function used: asyncexec"];()];
:();
];
Expand Down