Re: [Monetdb-developers] [Monetdb-sql-checkins] sql/src/backends/monet5 merovingian.mx, , 1.58, 1.59 monetdb.mx, , 1.27, 1.28
On 12-10-2008 10:31:31 +0000, Martin Kersten wrote:
Update of /cvsroot/monetdb/sql/src/backends/monet5 In directory 23jxhf1.ch3.sourceforge.com:/tmp/cvs-serv23781
Modified Files: merovingian.mx monetdb.mx Log Message: Use a known location for the lock file.
known?
- snprintf(lockfile, 512, "%s/.merovingian_lock", dbfarm); + /* the lock file is placed in the same directory as the pid */ + strcpy(lockfile, pidfilename); + c= strrchr(lockfile,'/'); + if (c) + strcpy(c+1,".merovingian_lock"); + } else + strcat(lockfile, ".merovingian_lock");
Why would you move out the lockfile from the dbfarm, and put it in /var/run instead? merovingian manages a dbfarm, hence the most obvious place to lock is the dbfarm itself. You now allow a user to use a different pidfile, but the same dbfarm and get two merovingians managing the same dbfarm, which seems like NOT a good idea to me. Below changes seem unrelated to the commit message to me. It indeed fixes one bug. However, if you change the input prompt to ignore an empty response, you probably want to change the documentation accordingly, as it states that the default is No, while you now require the user to at least type something. (Yes I'm picky, because everything was very well thought over.)
U monetdb.mx Index: monetdb.mx =================================================================== RCS file: /cvsroot/monetdb/sql/src/backends/monet5/monetdb.mx,v retrieving revision 1.27 retrieving revision 1.28 diff -u -d -r1.27 -r1.28 --- monetdb.mx 10 Oct 2008 08:54:15 -0000 1.27 +++ monetdb.mx 12 Oct 2008 10:31:29 -0000 1.28 @@ -24,7 +24,7 @@ The monetdb utility is designed to be the interface for the DBA to the dbfarm. Creating or deleting databases next to retrieving status information about them are the primary goals of this tool. - +@{ @h #define TOOLKIT_VERSION "0.4"
@@ -282,11 +282,13 @@ } /* demo | state | uptime | health */ secondsToString(avg, uplog.avguptime, 0); - printf("%-14s %s %12s %3d%%, %3s %s\n", - dbname, - state, uptime, - 100 - (uplog.crashcntr * 100 / uplog.startcntr), - avg, crash); + printf("%-14s %s %12s", + dbname, state, uptime); + if (uplog.startcntr) + printf(" %3d%%, %3s %s", + 100 - (uplog.crashcntr * 100 / uplog.startcntr), + avg, crash); + printf("\n"); } else if (mode == 2) { /* long mode */ char *state; @@ -1076,9 +1078,10 @@ printf("you are about to remove database '%s'\n", dbname); printf("ALL data in this database will get lost, " "are you sure? [y/N] "); - if (scanf("%c", &answ) >= 1 && (answ == 'y' || answ == 'Y')) { - /* do it! */ - } else { + answ = getchar(); + while (answ!= '\n' && getchar()!= '\n') + ; + if (answ != 'y' && answ != 'Y') { printf("battle control terminated\n"); exit(0); } @@ -1361,4 +1364,5 @@ return(0); }
+@} /* vim:set ts=4 sw=4 noexpandtab: */
------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ Monetdb-sql-checkins mailing list Monetdb-sql-checkins@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/monetdb-sql-checkins
Fabian Groffen wrote:
On 12-10-2008 10:31:31 +0000, Martin Kersten wrote:
Update of /cvsroot/monetdb/sql/src/backends/monet5 In directory 23jxhf1.ch3.sourceforge.com:/tmp/cvs-serv23781
Modified Files: merovingian.mx monetdb.mx Log Message: Use a known location for the lock file.
known?
- snprintf(lockfile, 512, "%s/.merovingian_lock", dbfarm); + /* the lock file is placed in the same directory as the pid */ + strcpy(lockfile, pidfilename); + c= strrchr(lockfile,'/'); + if (c) + strcpy(c+1,".merovingian_lock"); + } else + strcat(lockfile, ".merovingian_lock");
Why would you move out the lockfile from the dbfarm, and put it in /var/run instead? merovingian manages a dbfarm, hence the most obvious place to lock is the dbfarm itself.
You now allow a user to use a different pidfile, but the same dbfarm and get two merovingians managing the same dbfarm, which seems like NOT a good idea to me.
Below changes seem unrelated to the commit message to me. It indeed fixes one bug. However, if you change the input prompt to ignore an empty response, you probably want to change the documentation accordingly, as it states that the default is No, while you now require the user to at least type something.
(Yes I'm picky, because everything was very well thought over.)
U monetdb.mx Index: monetdb.mx =================================================================== RCS file: /cvsroot/monetdb/sql/src/backends/monet5/monetdb.mx,v retrieving revision 1.27 retrieving revision 1.28 diff -u -d -r1.27 -r1.28 --- monetdb.mx 10 Oct 2008 08:54:15 -0000 1.27 +++ monetdb.mx 12 Oct 2008 10:31:29 -0000 1.28 @@ -24,7 +24,7 @@ The monetdb utility is designed to be the interface for the DBA to the dbfarm. Creating or deleting databases next to retrieving status information about them are the primary goals of this tool. - +@{ @h #define TOOLKIT_VERSION "0.4"
@@ -282,11 +282,13 @@ } /* demo | state | uptime | health */ secondsToString(avg, uplog.avguptime, 0); - printf("%-14s %s %12s %3d%%, %3s %s\n", - dbname, - state, uptime, - 100 - (uplog.crashcntr * 100 / uplog.startcntr), - avg, crash); + printf("%-14s %s %12s", + dbname, state, uptime); + if (uplog.startcntr) + printf(" %3d%%, %3s %s", + 100 - (uplog.crashcntr * 100 / uplog.startcntr), + avg, crash); + printf("\n"); } else if (mode == 2) { /* long mode */ char *state; @@ -1076,9 +1078,10 @@ printf("you are about to remove database '%s'\n", dbname); printf("ALL data in this database will get lost, " "are you sure? [y/N] "); - if (scanf("%c", &answ) >= 1 && (answ == 'y' || answ == 'Y')) { - /* do it! */ - } else { + answ = getchar(); + while (answ!= '\n' && getchar()!= '\n') + ; + if (answ != 'y' && answ != 'Y') { printf("battle control terminated\n"); exit(0); } @@ -1361,4 +1364,5 @@ return(0); }
+@} /* vim:set ts=4 sw=4 noexpandtab: */
------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ Monetdb-sql-checkins mailing list Monetdb-sql-checkins@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/monetdb-sql-checkins
------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ Monetdb-developers mailing list Monetdb-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/monetdb-developers
yes, the roll of dbfarm might be deprecated as well. but most importantly, merovingian so far does not perform 'mkdir -p' which means dbfarm may not exist. This is the shortest fix.
On 12-10-2008 14:16:06 +0200, Martin Kersten wrote:
yes, the roll of dbfarm might be deprecated as well. but most importantly, merovingian so far does not perform 'mkdir -p' which means dbfarm may not exist. This is the shortest fix.
I already argued why the "fix" breaks, and doesn't fix the issue you first observed. Good luck with it.
participants (2)
-
Fabian Groffen
-
Martin Kersten