Skip to content

Conversation

@vldpyatkov
Copy link
Contributor

No description provided.

Comment on lines +578 to +600
tablesVv.update(causalityToken, previous -> {
var val = previous == null ? new HashMap() : new HashMap<>(previous);

val.put(name, table);

return val;
}, th -> {
throw new IgniteInternalException(IgniteStringFormatter.format("Cannot create a table [name={}, id={}]", name, tblId),
th);
});

tablesByIdVv.update(causalityToken, previous -> {
var val = previous == null ? new HashMap() : new HashMap<>(previous);

val.put(tblId, table);

return val;
}, th -> {
throw new IgniteInternalException(IgniteStringFormatter.format("Cannot create a table [name={}, id={}]", name, tblId),
th);
});

fireEvent(TableEvent.CREATE, new TableEventParameters(table), null);
completeApiCreateFuture(table);
Copy link
Contributor

Choose a reason for hiding this comment

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

the signature of update allows async completion, maybe we should rewrite these updates and final call of completeApiCreateFuture via thenCombine? This is also about dropTableLocally .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think, it might be implemented in a task of the update command.
Although we will even not finalize a token in the method, it will, should complete in storage revision update.

Comment on lines +615 to +622
CompletableFuture<Table> tblFut = tableCreateFuts.get(table.tableId());

if (tblFut != null) {
tblFut.complete(table);

tableCreateFuts.values().removeIf(fut -> fut == tblFut);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

what about using tableCreateFuts.remove to get the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This future can be repeated in the map (tableCreateFuts), because update closure may execute several times.
It is a tricky way to return a result after configuration update has handled.

}

var tbl = tablesById.get(id);
Map<UUID, TableImpl> tablesById = tablesByIdVv.get().join();
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure that such calls are NPE-safe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Below the line, I am checking the value to null.

@vldpyatkov vldpyatkov closed this Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants