Adar Dembo has posted comments on this change. Change subject: [security] load/store public TSK in the system table ......................................................................
Patch Set 23: > > Now that I'm seeing the TSK deletion code alongside the insertion > > code, I think it'd be a good idea to perform both operations in a > > single sys catalog write. So the flow could look something like > > this: > > > > <read all TSKs, keeping track of expired ones> > > tsk_actions.tsks_to_delete = expired_TSKs > > <check and generate new TSK> > > if new TSK needed: > > tsk_actions.tsks_to_add = new_TSK > > Write(tsk_actions) > > > > What do you think? > > This sounds good from the performance perspective, but it > introduces additional coupling. Also, imagine a situation when the > delete ops of the combined request succeeds, but the inserts fail. > Then we could end up in a situation when there is no TSK entries in > the system table at all, and we have non-persisted key in run-time. > The former is bad because the sequence of TSKs might be disrupted > when leadership switches no another master instance (its > TokenSigner will start with key sequence number 0 when there is TSK > seq_num 100500 in the system. The latter is also not so good > because the system could start using non-persisted TSK, and when > leadership switched to another master, that valid key could no > longer be seen. > > I would opt to keep it as is. Hmm. I agree that there's more coupling, though in this case it seems warranted. The way I look at it, all of this falls under "TSK maintenance", a task done at leader-switch time. Viewed that way, combining the two operations makes sense as they're semantically related. I don't think the failure you're describing is possible; isn't the schema designed in such a way that INSERT failures (due to duplicate keys) are impossible? Anyway, I don't feel strongly enough about it to warrant a change. -- To view, visit http://gerrit.cloudera.org:8080/5935 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie91d4129bda0ca49e81988c28385895a2abcd201 Gerrit-PatchSet: 23 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: No
