dataset locking and heartbeat #223

John-A-Davies · 2020-10-09T10:49:29Z

These changes provide support in the zss server for dataset edit with pessimistic locking. It calls ISGENQ in response to a new URI called /datasetEnqueue. The ENQ is held in a waiting process which ends when datasetEnqueue is called with the DELETE verb.

Signed-off-by: John Davies <daviesja@uk.ibm.com>

ifakhrutdinov

@John-A-Davies, please convert this into a draft.

ifakhrutdinov · 2020-12-01T12:33:11Z

.vscode/launch.json

@@ -0,0 +1,19 @@
+{


Why do we need this file?

I used this file a long time ago to try and launch some of the code in a VSCode debugger. I abandoned that, so I don't need the file.

I've deleted it.

ifakhrutdinov · 2020-12-01T12:33:36Z

build/build_env.sh

@@ -12,7 +12,7 @@ set -e
 ################################################################################


-export _C89_LSYSLIB="CEE.SCEELKED:SYS1.CSSLIB:CSF.SCSFMOD0"
+export _C89_LSYSLIB="CEE.SCEELKED:SYS1.CSSLIB:PP.CSF.ZOS203.SCSFMOD0"


Is this compatible with a typical LPAR?

No, it's a local change that I needed on my test LPAR. I don't want that change to be pushed to git.

I will revert line 15 to its original in Git. The build needs PP.CSF.ZOS203.SCSFMOD0 only on the target z/OS system.

ifakhrutdinov · 2020-12-01T12:34:03Z

c/datasetService.c

    char *l1 = stringListPrint(request->parsedFile, 1, 1, "/", 0);
    char *percentDecoded = cleanURLParamValue(response->slh, l1);
-    char *filenamep1 = stringConcatenate(response->slh, "//'", percentDecoded);
-    char *filename = stringConcatenate(response->slh, filenamep1, "'");
+    // char *datasetName = cleanURLParamValue(response->slh, l1);


Don't keep commented out code.

It's a work-in-progress

commented out code deleted

ifakhrutdinov · 2020-12-01T12:34:53Z

c/datasetService.c

+
+    /* append PID to filename */
+    pid_t pid = getpid();
+    char pid_string[10];


Please use the style the rest of the code uses. That is, use camel case.

code deleted

c/datasetService.c

ifakhrutdinov · 2020-12-01T12:38:44Z

@John-A-Davies you'll need to point the submodule to the zowe-common-c commit you'd like zss to use. When this is resolved the automation should build the branch. @jackjia-ibm can probably provide more info on that.

File was for visual debugger but is no longer needed

Revert to repo value, keep local file which has changes to dataset names on my test system

changes only needed locally

remove local changes

Signed-off-by: John Davies <daviesja@uk.ibm.com>

ifakhrutdinov

See my comments.

Additionally, a proper description of the pull-request is needed. The title isn't clear either. What does it mean?

ifakhrutdinov · 2020-12-09T15:16:13Z

build/build_env.sh

@@ -1,57 +0,0 @@
-#!/bin/sh


This file shouldn't have been removed.

Adding it back now.

ifakhrutdinov · 2020-12-09T15:17:18Z

c/datasetService.c

@@ -99,13 +99,16 @@ static int serveDatasetContents(HttpService *service, HttpResponse *response){
    respondWithDataset(response, filename, TRUE);
  }
  else if (!strcmp(request->method, methodPOST)){
+    /* this section is WIP to allow saving of members when another user


Should this be part of the pull-request if it's WIP?

I don't think that comment came from me. That ELSE block AT LINE 101 contains the same code as staging, apart from the comment, so I guess someone else removed the comment. I did add the EOL comments for my own interest on lines 106+107, but I made no other change to that ELSE block

I don't think that comment came from me.

Al the commits in this pull-request belong to you. I don't see who else could have added this comment.

Yes, you're right, the comment is mine. Now I remember. I added it as a placeholder to remind me that there was a problem to be fixed. So the comment refers to code I've not written yet.

The problem is, if a user has the PDS directory open, the POST code at line 101 won't work. This is because the POST accesses the dataset via USS. If you try it yourself at the USS command prompt, you will find that writing to a member of that PDS won't work while the directory is open.

I have a solution for this, but I've not had time to finalise it.

hopefully removed extraneous comments

ifakhrutdinov · 2020-12-09T15:17:37Z

c/datasetService.c

    zowelog(NULL, LOG_COMP_ID_MVD_SERVER, ZOWE_LOG_DEBUG, "Updating if exists: %s\n", filename);
    fflush(stdout);
-    updateDataset(response, filename, TRUE);
+    updateDataset(response, filename, TRUE);   


Don't add trailing space.

It's harmless in C. If there's a formatting standard, then we should have a code formatter to fix this sort of thing. Saves time.

It's a change that does nothing. We shouldn't commit trailing spaces.

The rule of thumb is to use the same style as the rest of the file.

ifakhrutdinov · 2020-12-09T15:19:02Z

c/datasetService.c

+  zowelog(NULL, LOG_COMP_ID_MVD_SERVER, ZOWE_LOG_DEBUG2, "begin %s\n", __FUNCTION__);
+
+  zowelog(NULL, LOG_COMP_ID_MVD_SERVER, ZOWE_LOG_DEBUG2, "%s\n", __FUNCTION__);


Why 2 almost identical calls?

Removed the 2nd one.

ifakhrutdinov · 2020-12-09T15:19:43Z

c/datasetService.c

+    char *filenamep1 = stringConcatenate(response->slh, "//'", percentDecoded);
+    char *filename = stringConcatenate(response->slh, filenamep1, "'");
+    zowelog(NULL, LOG_COMP_ID_MVD_SERVER, ZOWE_LOG_DEBUG, "Taking enqueue on: %s\n", filename);
+    fflush(stdout);


You don't need fflush with zowelog.

Understood; I'll remove the 2 I added. There are another 6 instances in that C file. I'm happy to remove them too.

ifakhrutdinov · 2020-12-09T15:22:05Z

c/datasetService.c

+    char *l1 = stringListPrint(request->parsedFile, 1, 1, "/", 0);
+    char *percentDecoded = cleanURLParamValue(response->slh, l1);
+    char *filenamep1 = stringConcatenate(response->slh, "//'", percentDecoded);
+    char *filename = stringConcatenate(response->slh, filenamep1, "'");


Can any of these be NULL?

Interesting question and good spot of a potential error case.

These 4 lines are used by serveDatasetContents, serveDatasetEnqueue (my new function) and serveVSAMDatasetContents, when the logic requires the DSN to be wrapped in the format //'dsn' for use by USS. If either of my 2 usages of these 4 lines produces an error, then so could the other 4 uses in serveDatasetContents and serveVSAMDatasetContents.

This error can only happen with the API, since the file editor always ensures that the DSN is not null. I don't know if the API will catch that before it gets to this C program.

I'd like @struga0258 to create a test for this case. If that test fails, then I suggest we create a new issue and fix it at that stage.

unchanged from original, need to review again, so far didnt encounter any error here, will ned to check edge cases like sending empty filename etc

ifakhrutdinov · 2020-12-09T15:22:46Z

c/datasetService.c

@@ -194,6 +247,19 @@ void installDatasetContentsService(HttpServer *server) {
  registerHttpService(server, httpService);
 }

+/* new */


Bad comment.

ifakhrutdinov · 2020-12-09T15:23:01Z

c/datasetService.c

+  zowelog(NULL, LOG_COMP_ID_MVD_SERVER, ZOWE_LOG_DEBUG, "Returning from %s\n", __FUNCTION__);
+  return 0;
+}
+/* end of new for ENQ */


No need to comment obvious things.

ifakhrutdinov · 2020-12-09T15:23:34Z

c/zis/build.sh

@@ -88,7 +88,7 @@ as "${ASFLAGS[@]}" -aegimrsx=nwm.asm nwm.s
 as "${ASFLAGS[@]}" -aegimrsx=secmgmt.asm secmgmt.s
 as "${ASFLAGS[@]}" -aegimrsx=snarfer.asm snarfer.s

-export _LD_SYSLIB="//'SYS1.CSSLIB'://'CEE.SCEELKEX'://'CEE.SCEELKED'://'CEE.SCEERUN'://'CEE.SCEERUN2'://'CSF.SCSFMOD0'"
+export _LD_SYSLIB="//'SYS1.CSSLIB'://'CEE.SCEELKEX'://'CEE.SCEELKED'://'CEE.SCEERUN'://'CEE.SCEERUN2'://'PP.CSF.ZOS203.SCSFMOD0'"


This shouldn't have been changed.

You're right; it was for the local build. I will revert it.

ifakhrutdinov · 2020-12-09T15:24:34Z

c/zss.c

@@ -666,6 +666,7 @@ static WebPluginListElt* readWebPluginDefinitions(HttpServer *server, ShortLived
              }
              int pluginLogLevel = checkLoggingVerbosity(serverConfigFile, identifier, slh);
              if (identifier && pluginLocation) {
+                zowelog(NULL, LOG_COMP_ID_MVD_SERVER, ZOWE_LOG_WARNING, "Plugin ID '%s' is being loaded.\n", identifier); /* debug */


Do you still need this?

ifakhrutdinov · 2020-12-09T15:29:07Z

@John-A-Davies, all the code changes (including cleanup) are supposed to be completed when a pull-request is open. Please convert it to a draft if you expect more work.

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

refactor semaphore and address feedback

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

1000TurquoisePogs · 2021-10-07T19:33:41Z

Thank you all for your work on pessimistic locking approach to writing datasets.
At this time we've merged optimistic locking (#355) as the pros-cons of it turned out favorable compared to pessimistic. We may come back to pessimistic locking if needed but for now I'm going to close such tickets and perhaps we'll revisit them.

John-A-Davies added 5 commits October 9, 2020 11:45

upload phase 2

2d7e4cc

separate enq service

8b9148e

Signed-off-by: John Davies <daviesja@uk.ibm.com>

phase 2 upload

a608961

Signed-off-by: John Davies <daviesja@uk.ibm.com>

Remove new variadic parms

c7fd9f7

Signed-off-by: John Davies <daviesja@uk.ibm.com>

update submodule

2ed8801

ifakhrutdinov requested changes Dec 1, 2020

View reviewed changes

John-A-Davies added 5 commits December 1, 2020 14:01

Delete launch.json

fe7a43b

File was for visual debugger but is no longer needed

Delete build_env.sh

afb45df

Revert to repo value, keep local file which has changes to dataset names on my test system

Delete build_zis.sh

db2ce52

changes only needed locally

Update build_zss.sh

03d6bac

remove local changes

Update build_zss.sh

a45c504

remove local changes

John-A-Davies mentioned this pull request Dec 1, 2020

Edit a dataset zowe/zlux#498

Closed

John-A-Davies added 4 commits December 2, 2020 15:29

stage the zowe-common-c changes

c14c563

Signed-off-by: John Davies <daviesja@uk.ibm.com>

Merge branch 'dsedit#498' of github.com:zowe/zss into dsedit#498

2d40b56

upload build_zis

d7cec55

Signed-off-by: John Davies <daviesja@uk.ibm.com>

cleanup per review comments, and BAU changes from staging

ab3a698

Signed-off-by: John Davies <daviesja@uk.ibm.com>

ifakhrutdinov requested changes Dec 9, 2020

View reviewed changes

John-A-Davies and others added 11 commits January 20, 2021 17:32

updates from review comments

ba6f02a

adding test suite initital commit, not tested yet

13e21e6

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

made test suite functioning

b652f4f

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

add dataset heartbeat

fa6543a

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

register background handler from zss.c

f1f246a

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

add new generic background handler

89649d8

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

GET to POST dataservice

f2c4390

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

make heartbeat expiry and loop time configurable

18992d9

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

update printf with zowelog

6a16b81

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

refactor semaphore and address feedback

e2cb765

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

Merge pull request #1 from NakulManchanda/dsedit#498_rev2

d5f5a75

refactor semaphore and address feedback

Nakul Manchanda added 28 commits March 11, 2021 01:37

increased encapsulation

06b163a

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

update zowe-coomon-c commit

fec7978

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

Merge branch 'staging' into dsedit#498

55d3958

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

updated tests, add some helper launch scripts

a66f49c

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

update stc background server reference

c252226

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

remove only from test

74307b1

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

zowe-commonc update

216c188

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

Merge branch 'staging' into dsedit#498

8a26045

merge staging c

f8a7d2d

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

Merge branch 'staging' into dsedit#498

deb4ed7

reset common-c

76be86a

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

refactor on stcBackground changes

1a970d5

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

update in sync with stcbackground handler

b64258d

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

Merge branch 'staging' into dsedit#498

f29f8f4

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

removed server reference from callback signature

1ef0687

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

Merge branch 'staging' into dsedit#498

5627349

update common dependency

0962dd5

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

add swagger url for convinience testing

8c28d99

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

move label to static

65256bb

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

update zowe common c

336baaf

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

update common c dep

65b1abd

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

update zowe-common-c

3d2cf27

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

Merge branch 'staging' into dsedit#498

cc6c903

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

merge staging update zowe-common-c

0379462

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

update zowe-common-c

6d58142

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

update zowe-common-c

58bf490

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

Merge branch 'staging' into dsedit#498

032e939

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

update zowe-common-c ref

2e3d3e6

Signed-off-by: Nakul Manchanda <nakul.manchanda@ibm.com>

1000TurquoisePogs closed this Oct 7, 2021

		zowelog(NULL, LOG_COMP_ID_MVD_SERVER, ZOWE_LOG_DEBUG2, "begin %s\n", __FUNCTION__);

		zowelog(NULL, LOG_COMP_ID_MVD_SERVER, ZOWE_LOG_DEBUG2, "%s\n", __FUNCTION__);

dataset locking and heartbeat #223

dataset locking and heartbeat #223

Conversation

John-A-Davies commented Oct 9, 2020 • edited Loading

ifakhrutdinov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

John-A-Davies Jan 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ifakhrutdinov commented Dec 1, 2020

ifakhrutdinov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ifakhrutdinov commented Dec 9, 2020 • edited Loading

1000TurquoisePogs commented Oct 7, 2021

John-A-Davies commented Oct 9, 2020 •

edited

Loading

John-A-Davies Jan 19, 2021 •

edited

Loading

ifakhrutdinov commented Dec 9, 2020 •

edited

Loading