コード最適化によるバグ
3885 ワード
朝早くサーバーが警報を出して、topはすぐに、システムのCPUの負荷はすでに15まで急騰して、アクセスすることができません!
jstack pid印刷スタックは、RUNNABLEでjsonのスレッドを大量に解析します.
以前はjson解析の効率のために既成のクラスライブラリを放棄して、手動でjson解析をしたことをふと思い出して、このようにした結果は効率が50倍以上向上して、その時はまだ長い間自慢していました!
まさか今日突然1本のフォーマットの合法的でないデータが、意外にも死の循環を招いたとは思わなかった!
汚れたデータがどのようにシステムに入ったのかはともかく、Jsonを解析するコードを見てみましょう.
注意1箇所と2箇所は、フォーマットが合法でない場合はそのまま次のループを行います.問題はここにあるsubstring(colonpos+1,commapos)が取り出したのは空で、次のループに入ると元の文字列は変更されず、次回も繰り返し実行され、デッドループを招きました!
低級なミス!Fuck!
教訓:
(1)コードを簡単に最適化しない.
(2)必ずユニットテストを行い、正常分岐と異常分岐、正常データと異常データを考慮する.
(3)CPU負荷が高すぎると,デッドサイクルが発生する可能性が高い!
ps:スタックを印刷する方法が表示されます.
jstack pid印刷スタックは、RUNNABLEでjsonのスレッドを大量に解析します.
以前はjson解析の効率のために既成のクラスライブラリを放棄して、手動でjson解析をしたことをふと思い出して、このようにした結果は効率が50倍以上向上して、その時はまだ長い間自慢していました!
まさか今日突然1本のフォーマットの合法的でないデータが、意外にも死の循環を招いたとは思わなかった!
汚れたデータがどのようにシステムに入ったのかはともかく、Jsonを解析するコードを見てみましょう.
public static List getPartneridsFromJson(String data){
//{\"data\":[{\"partnerid\":982,\"count\":\"10000\",\"cityid\":\"11\"},{\"partnerid\":983,\"count\":\"10000\",\"cityid\":\"11\"},{\"partnerid\":984,\"count\":\"10000\",\"cityid\":\"11\"}]}
//
List list = new ArrayList(2);
if(data == null || data.length() <= 0){
return list;
}
int datapos = data.indexOf("data");
if(datapos < 0){
return list;
}
int leftBracket = data.indexOf("[",datapos);
int rightBracket= data.indexOf("]",datapos);
if(leftBracket < 0 || rightBracket < 0){
return list;
}
String partners = data.substring(leftBracket+1,rightBracket);
if(partners == null || partners.length() <= 0){
return list;
}
while(partners!=null && partners.length() > 0){
int idpos = partners.indexOf("partnerid");
if(idpos < 0){
break;
}
int colonpos = partners.indexOf(":",idpos);
int commapos = partners.indexOf(",",idpos);
if(colonpos < 0 || commapos < 0){
//partners = partners.substring(idpos+"partnerid".length());//1
continue;
}
String pid = partners.substring(colonpos+1,commapos);
if(pid == null || pid.length() <= 0){
//partners = partners.substring(idpos+"partnerid".length());//2
continue;
}
try{
list.add(Long.parseLong(pid));
}catch(Exception e){
//do nothing
}
partners = partners.substring(commapos);
}
return list;
}
注意1箇所と2箇所は、フォーマットが合法でない場合はそのまま次のループを行います.問題はここにあるsubstring(colonpos+1,commapos)が取り出したのは空で、次のループに入ると元の文字列は変更されず、次回も繰り返し実行され、デッドループを招きました!
低級なミス!Fuck!
教訓:
(1)コードを簡単に最適化しない.
(2)必ずユニットテストを行い、正常分岐と異常分岐、正常データと異常データを考慮する.
(3)CPU負荷が高すぎると,デッドサイクルが発生する可能性が高い!
ps:スタックを印刷する方法が表示されます.
:
public static java.util.List list_threads(){
int tc = Thread.activeCount();
Thread[] ts = new Thread[tc];
Thread.enumerate(ts);
return java.util.Arrays.asList(ts);
}
@Path(value = "/api/stackinfo")
@GET
public ActionResult stackinfo() {
String str = "Memory:";
str +="";
str +="- freeMemory="+Runtime.getRuntime().freeMemory()/(1024*1024)+"M
";
str +="- totalMemory="+Runtime.getRuntime().totalMemory()/(1024*1024)+"M
";
str +="- maxMemory="+Runtime.getRuntime().maxMemory()/(1024*1024)+"M
";
str +="
";
str +="
";
str +="Thread:";
str +="";
for(Thread t : list_threads()){
str +="- "+t.getName()+","+t.getState()+":"+ t.getClass().getName()+"
";
StackTraceElement[] elems = t.getStackTrace();
str += "";
for(StackTraceElement elem : elems){
str +="- "+elem.toString()+"
";
}
str += "
";
}
str +="
";
beat.getResponse().setContentType("text/html;charset=UTF-8");
return new ContentResult(str);
}