Skip to content
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.

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

refact(core): remove lock of globalMasterInfo to optimize perf #2151

Merged
merged 2 commits into from
Mar 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -76,23 +76,22 @@ public void filter(ContainerRequestContext context) throws IOException {
E.checkState(handle != null, "Context GraphManager is absent");
GraphManager manager = handle.getService();
E.checkState(manager != null, "Context GraphManager is absent");
GlobalMasterInfo globalMasterInfo = manager.globalMasterInfo();
if (globalMasterInfo == null || !globalMasterInfo.isFeatureSupport()) {
return;
}

String redirectTag = context.getHeaderString(X_HG_REDIRECT);
if (StringUtils.isNotEmpty(redirectTag)) {
return;
}

String url;
synchronized (globalMasterInfo) {
if (globalMasterInfo.isMaster() || StringUtils.isEmpty(globalMasterInfo.url())) {
return;
}
url = globalMasterInfo.url();
GlobalMasterInfo globalMasterInfo = manager.globalMasterInfo();
if (globalMasterInfo == null || !globalMasterInfo.isFeatureSupport()) {
return;
}
GlobalMasterInfo.NodeInfo masterNodeInfo = globalMasterInfo.nodeInfo();
if (masterNodeInfo == null || masterNodeInfo.isMaster() ||
StringUtils.isEmpty(masterNodeInfo.url())) {
return;
}
String url = masterNodeInfo.url();

URI redirectUri;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,22 @@

public class GlobalMasterInfo {

private boolean isMaster;
private String url;

private NodeInfo nodeInfo;
private volatile boolean featureSupport;

public GlobalMasterInfo() {
this.featureSupport = false;
this.nodeInfo = new NodeInfo(false, "");
}

public synchronized void set(boolean isMaster, String url) {
this.isMaster = isMaster;
this.url = url;
}

public synchronized boolean isMaster() {
return this.isMaster;
public final void nodeInfo(boolean isMaster, String url) {
Copy link
Member

Choose a reason for hiding this comment

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

nice but why do we add final for the method?(for design?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice but why do we add final for the method?(for design?)

final method no usefull,only express dont modify

Copy link
Contributor

Choose a reason for hiding this comment

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

looks useful, the subclasses cannot override it

// final can avoid instruction rearrangement, visibility can be ignored
final NodeInfo tmp = new NodeInfo(isMaster, url);
this.nodeInfo = tmp;
}

public synchronized String url() {
return this.url;
public final NodeInfo nodeInfo() {
return this.nodeInfo;
}

public void isFeatureSupport(boolean featureSupport) {
Expand All @@ -48,4 +44,23 @@ public void isFeatureSupport(boolean featureSupport) {
public boolean isFeatureSupport() {
return this.featureSupport;
}

public static class NodeInfo {

private final boolean isMaster;
private final String url;

public NodeInfo(boolean isMaster, String url) {
this.isMaster = isMaster;
this.url = url;
}

public boolean isMaster() {
return this.isMaster;
}

public String url() {
return this.url;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,12 @@ public void error(StateMachineContext context, Throwable e) {
public void initGlobalMasterInfo(StateMachineContext context) {
StateMachineContext.MasterServerInfo master = context.master();
if (master == null) {
this.globalMasterInfo.set(false, null);
this.globalMasterInfo.nodeInfo(false, "");
return;
}

boolean isMaster = Objects.equals(context.node(), master.node());
String url = master.url();
this.globalMasterInfo.set(isMaster, url);
this.globalMasterInfo.nodeInfo(isMaster, url);
}
}